Add experimental rules condition-wrapping and `mixed-condition-oper…

…ators` (#2401) * Add experimental ConditionWrappingRule and MixedConditionOperatorsRule
pinterest · Dec 2, 2023 · 1868713 · 1868713
1 parent 647e795
commit 1868713
Show file tree

Hide file tree

Showing 17 changed files with 722 additions and 85 deletions.
diff --git a/documentation/snapshot/docs/rules/experimental.md b/documentation/snapshot/docs/rules/experimental.md
@@ -333,6 +333,39 @@ The other code styles allow an infinite amount of parameters on the same line (a
 
 Rule id: `class-signature` (`standard` rule set)
 
+## Condition wrapping
+
+Wraps each operand in a multiline condition to a separate line.
+
+=== "[:material-heart:](#) Ktlint"
+
+    ```kotlin
+    val foo = bar || baz
+    if (bar1 ||
+        bar2 ||
+        baz1 ||
+        (baz2 && baz3)
+    ) {
+       // do somthing
+    }
+    ```
+
+=== "[:material-heart-off-outline:](#) Disallowed"
+
+    ```kotlin
+    val foo =
+      multiLineOperand(
+          "bar"
+      ) || baz
+    if (bar1 || bar2 ||
+        baz1 || (baz2 && baz3)
+    ) {
+       // do somthing
+    }
+    ```
+
+Rule id: `condition-wrapping` (`standard` rule set)
+
 ## Function expression body
 
 Rewrites a function body only containing a `return` or `throw` expression to an expression body.
@@ -478,6 +511,27 @@ Enforce a single whitespace between the modifier list and the function type.
 
 Rule id: `function-type-modifier-spacing` (`standard` rule set)
 
+## Mixed condition operators
+
+Conditions should not use a both `&&` and `||` operators between operators at the same level. By using parenthesis the expression is to be clarified.
+
+=== "[:material-heart:](#) Ktlint"
+
+    ```kotlin
+    val foo = bar1 && (bar2 || bar3) && bar4
+    ```
+
+=== "[:material-heart-off-outline:](#) Disallowed"
+
+    ```kotlin
+    val foo = bar1 &&
+        bar2 ||
+        bar3
+    val foo = bar1 && (bar2 || bar3 && bar4) && bar5
+    ```
+
+Rule id: `mixed-condition-operators` (`standard` rule set)
+
 ## Multiline loop
 
 Braces required for multiline for, while, and do statements.

diff --git a/ktlint-ruleset-standard/api/ktlint-ruleset-standard.api b/ktlint-ruleset-standard/api/ktlint-ruleset-standard.api
@@ -133,6 +133,16 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/CommentWrappingRu
 	public static final fun getCOMMENT_WRAPPING_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
 }
 
+public final class com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRule : com/pinterest/ktlint/ruleset/standard/StandardRule, com/pinterest/ktlint/rule/engine/core/api/Rule$Experimental {
+	public fun <init> ()V
+	public fun beforeFirstNode (Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfig;)V
+	public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZLkotlin/jvm/functions/Function3;)V
+}
+
+public final class com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRuleKt {
+	public static final fun getCONDITION_WRAPPING_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
+}
+
 public final class com/pinterest/ktlint/ruleset/standard/rules/ContextReceiverWrappingRule : com/pinterest/ktlint/ruleset/standard/StandardRule {
 	public fun <init> ()V
 	public fun beforeFirstNode (Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfig;)V
@@ -369,6 +379,15 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/MaxLineLengthRule
 	public static final fun getMAX_LINE_LENGTH_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
 }
 
+public final class com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRule : com/pinterest/ktlint/ruleset/standard/StandardRule, com/pinterest/ktlint/rule/engine/core/api/Rule$Experimental {
+	public fun <init> ()V
+	public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZLkotlin/jvm/functions/Function3;)V
+}
+
+public final class com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRuleKt {
+	public static final fun getMIXED_CONDITION_OPERATORS_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
+}
+
 public final class com/pinterest/ktlint/ruleset/standard/rules/ModifierListSpacingRule : com/pinterest/ktlint/ruleset/standard/StandardRule {
 	public fun <init> ()V
 	public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZLkotlin/jvm/functions/Function3;)V

diff --git a/...standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt b/...standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt
@@ -15,6 +15,7 @@ import com.pinterest.ktlint.ruleset.standard.rules.ClassNamingRule
 import com.pinterest.ktlint.ruleset.standard.rules.ClassSignatureRule
 import com.pinterest.ktlint.ruleset.standard.rules.CommentSpacingRule
 import com.pinterest.ktlint.ruleset.standard.rules.CommentWrappingRule
+import com.pinterest.ktlint.ruleset.standard.rules.ConditionWrappingRule
 import com.pinterest.ktlint.ruleset.standard.rules.ContextReceiverWrappingRule
 import com.pinterest.ktlint.ruleset.standard.rules.DiscouragedCommentLocationRule
 import com.pinterest.ktlint.ruleset.standard.rules.EnumEntryNameCaseRule
@@ -36,6 +37,7 @@ import com.pinterest.ktlint.ruleset.standard.rules.ImportOrderingRule
 import com.pinterest.ktlint.ruleset.standard.rules.IndentationRule
 import com.pinterest.ktlint.ruleset.standard.rules.KdocWrappingRule
 import com.pinterest.ktlint.ruleset.standard.rules.MaxLineLengthRule
+import com.pinterest.ktlint.ruleset.standard.rules.MixedConditionOperatorsRule
 import com.pinterest.ktlint.ruleset.standard.rules.ModifierListSpacingRule
 import com.pinterest.ktlint.ruleset.standard.rules.ModifierOrderRule
 import com.pinterest.ktlint.ruleset.standard.rules.MultiLineIfElseRule
@@ -108,6 +110,7 @@ public class StandardRuleSetProvider : RuleSetProviderV3(RuleSetId.STANDARD) {
             RuleProvider { ChainWrappingRule() },
             RuleProvider { ClassNamingRule() },
             RuleProvider { ClassSignatureRule() },
+            RuleProvider { ConditionWrappingRule() },
             RuleProvider { CommentSpacingRule() },
             RuleProvider { CommentWrappingRule() },
             RuleProvider { ContextReceiverWrappingRule() },
@@ -131,6 +134,7 @@ public class StandardRuleSetProvider : RuleSetProviderV3(RuleSetId.STANDARD) {
             RuleProvider { IndentationRule() },
             RuleProvider { KdocWrappingRule() },
             RuleProvider { MaxLineLengthRule() },
+            RuleProvider { MixedConditionOperatorsRule() },
             RuleProvider { ModifierListSpacingRule() },
             RuleProvider { ModifierOrderRule() },
             RuleProvider { MultiLineIfElseRule() },

diff --git a/...d/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt b/...d/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ArgumentListWrappingRule.kt
@@ -1,9 +1,16 @@
 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.BINARY_EXPRESSION
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.COLLECTION_LITERAL_EXPRESSION
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.DOT_QUALIFIED_EXPRESSION
 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.LPAR
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.RPAR
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.TYPE_ARGUMENT_LIST
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_ARGUMENT
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_ARGUMENT_LIST
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.WHITE_SPACE
 import com.pinterest.ktlint.rule.engine.core.api.IndentConfig
 import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule
 import com.pinterest.ktlint.rule.engine.core.api.Rule.VisitorModifier.RunAfterRule.Mode.ONLY_WHEN_RUN_AFTER_RULE_IS_LOADED_AND_ENABLED
@@ -101,11 +108,11 @@ public class ArgumentListWrappingRule :
 
     private fun needToWrapArgumentList(node: ASTNode) =
         if ( // skip when there are no arguments
-            node.firstChildNode?.treeNext?.elementType != ElementType.RPAR &&
+            node.firstChildNode?.treeNext?.elementType != RPAR &&
             // skip lambda arguments
-            node.treeParent?.elementType != ElementType.FUNCTION_LITERAL &&
+            node.treeParent?.elementType != FUNCTION_LITERAL &&
             // skip if number of arguments is big (we assume it with a magic number of 8)
-            node.children().count { it.elementType == ElementType.VALUE_ARGUMENT } <= 8 &&
+            node.children().count { it.elementType == VALUE_ARGUMENT } <= 8 &&
             // skip if part of a value argument list. It depends on the situation whether it is better to wrap the arguments in the list
             // or the operators in the binary expression
             !node.isPartOf(BINARY_EXPRESSION)
@@ -161,7 +168,7 @@ public class ArgumentListWrappingRule :
                 it
             }
         }.let {
-            if (child.elementType == ElementType.VALUE_ARGUMENT) {
+            if (child.elementType == VALUE_ARGUMENT) {
                 it + 1
             } else {
                 it
@@ -180,7 +187,7 @@ public class ArgumentListWrappingRule :
         autoCorrect: Boolean,
     ) {
         when (child.elementType) {
-            ElementType.LPAR -> {
+            LPAR -> {
                 val prevLeaf = child.prevLeaf()
                 if (prevLeaf is PsiWhiteSpace && prevLeaf.textContains('\n')) {
                     emit(child.startOffset, errorMessage(child), true)
@@ -189,8 +196,8 @@ public class ArgumentListWrappingRule :
                     }
                 }
             }
-            ElementType.VALUE_ARGUMENT,
-            ElementType.RPAR,
+            VALUE_ARGUMENT,
+            RPAR,
             -> {
                 // aiming for
                 // ... LPAR
@@ -228,31 +235,36 @@ public class ArgumentListWrappingRule :
 
     private fun errorMessage(node: ASTNode) =
         when (node.elementType) {
-            ElementType.LPAR -> """Unnecessary newline before "(""""
-            ElementType.VALUE_ARGUMENT ->
-                "Argument should be on a separate line (unless all arguments can fit a single line)"
-            ElementType.RPAR -> """Missing newline before ")""""
+            LPAR -> """Unnecessary newline before "(""""
+            VALUE_ARGUMENT -> "Argument should be on a separate line (unless all arguments can fit a single line)"
+            RPAR -> """Missing newline before ")""""
             else -> throw UnsupportedOperationException()
         }
 
     private fun ASTNode.textContainsIgnoringLambda(char: Char): Boolean =
         children().any { child ->
-            val elementType = child.elementType
-            elementType == ElementType.WHITE_SPACE && child.textContains(char) ||
-                elementType == ElementType.COLLECTION_LITERAL_EXPRESSION && child.textContains(char) ||
-                elementType == ElementType.VALUE_ARGUMENT && child.children().any { it.textContainsIgnoringLambda(char) }
+            child.isWhitespaceContaining(char) ||
+                child.isCollectionLiteralContaining(char) ||
+                child.isValueArgumentContaining(char)
         }
 
+    private fun ASTNode.isWhitespaceContaining(char: Char) = elementType == WHITE_SPACE && textContains(char)
+
+    private fun ASTNode.isCollectionLiteralContaining(char: Char) = elementType == COLLECTION_LITERAL_EXPRESSION && textContains(char)
+
+    private fun ASTNode.isValueArgumentContaining(char: Char) =
+        elementType == VALUE_ARGUMENT && children().any { it.textContainsIgnoringLambda(char) }
+
     private fun ASTNode.hasTypeArgumentListInFront(): Boolean =
         treeParent
             .children()
-            .firstOrNull { it.elementType == ElementType.TYPE_ARGUMENT_LIST }
+            .firstOrNull { it.elementType == TYPE_ARGUMENT_LIST }
             ?.children()
             ?.any { it.isWhiteSpaceWithNewline() } == true
 
     private fun ASTNode.isPartOfDotQualifiedAssignmentExpression(): Boolean =
         treeParent?.treeParent?.elementType == BINARY_EXPRESSION &&
-            treeParent?.treeParent?.children()?.find { it.elementType == ElementType.DOT_QUALIFIED_EXPRESSION } != null
+            treeParent?.treeParent?.children()?.find { it.elementType == DOT_QUALIFIED_EXPRESSION } != null
 
     private fun ASTNode.prevWhiteSpaceWithNewLine(): ASTNode? {
         var prev = prevLeaf()

diff --git a/...rc/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ChainMethodContinuationRule.kt b/...rc/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ChainMethodContinuationRule.kt
@@ -179,10 +179,7 @@ public class ChainMethodContinuationRule :
                 true
             }
 
-            chainOperators
-                .first()
-                .prevCodeSibling()
-                ?.elementType == STRING_TEMPLATE && !hasNewlineAfterLastChainOperator -> {
+            isChainedExpressionOnStringTemplate() && !hasNewlineAfterLastChainOperator -> {
                 // Allow:
                 //     """
                 //     some text
@@ -203,6 +200,13 @@ public class ChainMethodContinuationRule :
             else -> false
         }
 
+    private fun ChainedExpression.isChainedExpressionOnStringTemplate() =
+        STRING_TEMPLATE ==
+            chainOperators
+                .first()
+                .prevCodeSibling()
+                ?.elementType
+
     private fun ChainedExpression.exceedsMaxLineLength() =
         with(rootASTNode) {
             if (treeParent.elementType == BINARY_EXPRESSION) {