From 7317e9b74760d05d7a66d612ac9cfe996a429bab Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Wed, 31 May 2023 01:40:21 +0530 Subject: [PATCH 1/3] Fix false -ve on else with single line binary or chained call --- .../standard/rules/MultiLineIfElseRule.kt | 10 ++- .../standard/rules/MultiLineIfElseRuleTest.kt | 81 +++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt index 8434e2b1bb..5cf6932a78 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt @@ -64,7 +64,10 @@ public class MultiLineIfElseRule : return } - if (node.elementType == ELSE && node.firstChildNode?.elementType == BINARY_EXPRESSION) { + if (node.elementType == ELSE && + node.firstChildNode?.elementType == BINARY_EXPRESSION && + node.firstChildNode.firstChildNode?.elementType == IF + ) { // Allow // val foo = if (bar1) { // "bar1" @@ -74,7 +77,10 @@ public class MultiLineIfElseRule : return } - if (node.elementType == ELSE && node.firstChildNode?.elementType == DOT_QUALIFIED_EXPRESSION) { + if (node.elementType == ELSE && + node.firstChildNode?.elementType == DOT_QUALIFIED_EXPRESSION && + node.firstChildNode.firstChildNode?.elementType == IF + ) { // Allow // val foo = if (bar1) { // "bar1" diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt index e509735182..ebaf4ee219 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt @@ -579,14 +579,50 @@ class MultiLineIfElseRuleTest { } else { null } ?: "something-else" + + val foo2 = if (bar1) { + "bar1" + } else if (bar2) { + null + } else { + null + } ?: "something-else" """.trimIndent() multiLineIfElseRuleAssertThat(code).hasNoLintViolations() } + @Test + fun `Issue 2057 - Given an else condition with single line binary expression`() { + val code = + """ + val foo = if (bar1) { + "bar1" + } else bar2 ?: "something-else" + """.trimIndent() + val formattedCode = + """ + val foo = if (bar1) { + "bar1" + } else { + bar2 ?: "something-else" + } + """.trimIndent() + multiLineIfElseRuleAssertThat(code) + .hasLintViolations( + LintViolation(3, 8, "Missing { ... }"), + ).isFormattedAs(formattedCode) + } + @Test fun `Issue 1904 - Given an nested if else statement and else which is part of a dot qualified expression`() { val code = """ + val foo = if (bar1) { + "bar1" + } else { + "bar2" + }.plus("foo") + val foo = if (bar1) { "bar1" } else if (bar2) { @@ -597,4 +633,49 @@ class MultiLineIfElseRuleTest { """.trimIndent() multiLineIfElseRuleAssertThat(code).hasNoLintViolations() } + + @Test + fun `Issue 2057 - Given an else with chained condition`() { + val code = + """ + val a = if (System.currentTimeMillis() % 2 == 0L) { + 0 + } else System.currentTimeMillis().toInt() + """.trimIndent() + val formattedCode = + """ + val a = if (System.currentTimeMillis() % 2 == 0L) { + 0 + } else { + System.currentTimeMillis().toInt() + } + """.trimIndent() + multiLineIfElseRuleAssertThat(code) + .hasLintViolations( + LintViolation(3, 8, "Missing { ... }"), + ).isFormattedAs(formattedCode) + } + + @Test + fun `Issue 2057 - Given an if with chained condition`() { + val code = + """ + val a = if (System.currentTimeMillis() % 2 == 0L) System.currentTimeMillis().toInt() + else { + System.currentTimeMillis().toInt() + } + """.trimIndent() + val formattedCode = + """ + val a = if (System.currentTimeMillis() % 2 == 0L) { + System.currentTimeMillis().toInt() + } else { + System.currentTimeMillis().toInt() + } + """.trimIndent() + multiLineIfElseRuleAssertThat(code) + .hasLintViolations( + LintViolation(1, 51, "Missing { ... }"), + ).isFormattedAs(formattedCode) + } } From 2f164864a88efd7b85e1f510e3cf5ec51db5dc8c Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Wed, 31 May 2023 01:44:39 +0530 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3a3166d0e..9ece25e94d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ At this point in time, it is not yet decided what the next steps will be. Ktlint * Do not merge an annotated expression body with the function signature even if it fits on a single line ([#2043](https://github.com/pinterest/ktlint/issues/2043)) * Ignore property with name `serialVersionUID` in `property-naming` ([#2045](https://github.com/pinterest/ktlint/issues/2045)) * Prevent incorrect reporting of violations in case a nullable function type reference exceeds the maximum line length `parameter-list-wrapping` ([#1324](https://github.com/pinterest/ktlint/issues/1324)) +* Prevent false negative on `else` branch when body contains only chained calls or binary expression ([#2057](https://github.com/pinterest/ktlint/issues/2057)) ### Changed From cdea42c9b1ec6dc74388faac94605cb99b09b63a Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Sat, 3 Jun 2023 04:03:16 +0530 Subject: [PATCH 3/3] Update the comment code example Update the naming of dummy code in TC to be consistent --- .../standard/rules/MultiLineIfElseRule.kt | 24 +++++++++++-------- .../standard/rules/MultiLineIfElseRuleTest.kt | 16 ++++++------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt index 5cf6932a78..08f81d548a 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRule.kt @@ -69,11 +69,13 @@ public class MultiLineIfElseRule : node.firstChildNode.firstChildNode?.elementType == IF ) { // Allow - // val foo = if (bar1) { - // "bar1" - // } else { - // null - // } ?: "something-else" + // val foo2 = if (bar1) { + // "bar1" + // } else if (bar2) { + // null + // } else { + // null + // } ?: "something-else" return } @@ -82,11 +84,13 @@ public class MultiLineIfElseRule : node.firstChildNode.firstChildNode?.elementType == IF ) { // Allow - // val foo = if (bar1) { - // "bar1" - // } else { - // "bar2" - // }.plus("foo") + // val foo = if (bar1) { + // "bar1" + // } else if (bar2) { + // "bar2" + // } else { + // "bar3" + // }.plus("foo") return } diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt index ebaf4ee219..3f973f3474 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultiLineIfElseRuleTest.kt @@ -574,7 +574,7 @@ class MultiLineIfElseRuleTest { fun `Issue 1904 - Given an nested if else statement followed by an elvis operator`() { val code = """ - val foo = if (bar1) { + val foo1 = if (bar1) { "bar1" } else { null @@ -617,13 +617,13 @@ class MultiLineIfElseRuleTest { fun `Issue 1904 - Given an nested if else statement and else which is part of a dot qualified expression`() { val code = """ - val foo = if (bar1) { + val foo1 = if (bar1) { "bar1" } else { "bar2" }.plus("foo") - val foo = if (bar1) { + val foo2 = if (bar1) { "bar1" } else if (bar2) { "bar2" @@ -638,13 +638,13 @@ class MultiLineIfElseRuleTest { fun `Issue 2057 - Given an else with chained condition`() { val code = """ - val a = if (System.currentTimeMillis() % 2 == 0L) { + val foo = if (System.currentTimeMillis() % 2 == 0L) { 0 } else System.currentTimeMillis().toInt() """.trimIndent() val formattedCode = """ - val a = if (System.currentTimeMillis() % 2 == 0L) { + val foo = if (System.currentTimeMillis() % 2 == 0L) { 0 } else { System.currentTimeMillis().toInt() @@ -660,14 +660,14 @@ class MultiLineIfElseRuleTest { fun `Issue 2057 - Given an if with chained condition`() { val code = """ - val a = if (System.currentTimeMillis() % 2 == 0L) System.currentTimeMillis().toInt() + val foo = if (System.currentTimeMillis() % 2 == 0L) System.currentTimeMillis().toInt() else { System.currentTimeMillis().toInt() } """.trimIndent() val formattedCode = """ - val a = if (System.currentTimeMillis() % 2 == 0L) { + val foo = if (System.currentTimeMillis() % 2 == 0L) { System.currentTimeMillis().toInt() } else { System.currentTimeMillis().toInt() @@ -675,7 +675,7 @@ class MultiLineIfElseRuleTest { """.trimIndent() multiLineIfElseRuleAssertThat(code) .hasLintViolations( - LintViolation(1, 51, "Missing { ... }"), + LintViolation(1, 53, "Missing { ... }"), ).isFormattedAs(formattedCode) } }