From 1868713a760754a87fd4284a84089c1c124c571a Mon Sep 17 00:00:00 2001 From: Paul Dingemans Date: Sat, 2 Dec 2023 14:35:09 +0100 Subject: [PATCH] Add experimental rules `condition-wrapping` and `mixed-condition-operators` (#2401) * Add experimental ConditionWrappingRule and MixedConditionOperatorsRule --- .../snapshot/docs/rules/experimental.md | 54 ++++ .../api/ktlint-ruleset-standard.api | 19 ++ .../standard/StandardRuleSetProvider.kt | 4 + .../rules/ArgumentListWrappingRule.kt | 48 ++-- .../rules/ChainMethodContinuationRule.kt | 12 +- .../standard/rules/ConditionWrappingRule.kt | 138 ++++++++++ .../standard/rules/IfElseWrappingRule.kt | 12 +- .../standard/rules/ImportOrderingRule.kt | 24 +- .../rules/MixedConditionOperatorsRule.kt | 82 ++++++ .../NoEmptyFirstLineInMethodBlockRule.kt | 15 +- .../standard/rules/NoSemicolonsRule.kt | 3 +- .../standard/rules/NoUnusedImportsRule.kt | 3 +- .../standard/rules/SpacingAroundCurlyRule.kt | 63 ++--- .../rules/SpacingAroundOperatorsRule.kt | 16 +- .../standard/rules/SpacingAroundParensRule.kt | 10 +- .../rules/ConditionWrappingRuleTest.kt | 252 ++++++++++++++++++ .../rules/MixedConditionOperatorsRuleTest.kt | 52 ++++ 17 files changed, 722 insertions(+), 85 deletions(-) create mode 100644 ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRule.kt create mode 100644 ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRule.kt create mode 100644 ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRuleTest.kt create mode 100644 ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRuleTest.kt diff --git a/documentation/snapshot/docs/rules/experimental.md b/documentation/snapshot/docs/rules/experimental.md index b92c170b4c..c40dcc7524 100644 --- 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 index eef3a3e4ca..309354f79c 100644 --- 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 ()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 ()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 ()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 ()V public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;ZLkotlin/jvm/functions/Function3;)V diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt index c0bcf400e3..6bd36f8d3d 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/StandardRuleSetProvider.kt +++ b/ktlint-ruleset-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/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 8e02ed1609..087df09abb 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 @@ -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/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ChainMethodContinuationRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ChainMethodContinuationRule.kt index 0730fc3c16..7e199b9684 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ChainMethodContinuationRule.kt +++ b/ktlint-ruleset-standard/src/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) { diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRule.kt new file mode 100644 index 0000000000..f0db48210f --- /dev/null +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRule.kt @@ -0,0 +1,138 @@ +package com.pinterest.ktlint.ruleset.standard.rules + +import com.pinterest.ktlint.rule.engine.core.api.ElementType.ANDAND +import com.pinterest.ktlint.rule.engine.core.api.ElementType.BINARY_EXPRESSION +import com.pinterest.ktlint.rule.engine.core.api.ElementType.OPERATION_REFERENCE +import com.pinterest.ktlint.rule.engine.core.api.ElementType.OROR +import com.pinterest.ktlint.rule.engine.core.api.IndentConfig +import com.pinterest.ktlint.rule.engine.core.api.Rule +import com.pinterest.ktlint.rule.engine.core.api.RuleId +import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint +import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.EXPERIMENTAL +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.INDENT_SIZE_PROPERTY +import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPERTY +import com.pinterest.ktlint.rule.engine.core.api.firstChildLeafOrSelf +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.isWhiteSpaceWithNewline +import com.pinterest.ktlint.rule.engine.core.api.lastChildLeafOrSelf +import com.pinterest.ktlint.rule.engine.core.api.leavesInClosedRange +import com.pinterest.ktlint.rule.engine.core.api.nextSibling +import com.pinterest.ktlint.rule.engine.core.api.parent +import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe +import com.pinterest.ktlint.ruleset.standard.StandardRule +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet + +/** + * Wraps a condition (a boolean binary expression) whenever the expression does not fit on the line. In addition to the + * `binary-expression-wrapping`, operands are being wrapped in multiline expressions. + */ +@SinceKtlint("1.1.0", EXPERIMENTAL) +public class ConditionWrappingRule : + StandardRule( + id = "condition-wrapping", + usesEditorConfigProperties = + setOf( + INDENT_SIZE_PROPERTY, + INDENT_STYLE_PROPERTY, + ), + ), + Rule.Experimental { + private var indentConfig = IndentConfig.DEFAULT_INDENT_CONFIG + + override fun beforeFirstNode(editorConfig: EditorConfig) { + indentConfig = + IndentConfig( + indentStyle = editorConfig[INDENT_STYLE_PROPERTY], + tabWidth = editorConfig[INDENT_SIZE_PROPERTY], + ) + } + + override fun beforeVisitChildNodes( + node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, + ) { + node + .takeIf { it.isLogicalBinaryExpression() } + ?.takeIf { it.isMultiline() } + ?.let { + visitLogicalExpression(it, emit, autoCorrect) + } + } + + private fun ASTNode.isLogicalBinaryExpression() = + takeIf { elementType == BINARY_EXPRESSION } + ?.findChildByType(OPERATION_REFERENCE) + ?.let { it.firstChildNode.elementType in logicalOperators } + ?: false + + private fun visitLogicalExpression( + node: ASTNode, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, + autoCorrect: Boolean, + ) { + node + .findChildByType(OPERATION_REFERENCE) + ?.takeUnless { it.nextSibling().isWhiteSpaceWithNewline() } + ?.let { operationReference -> + val startOffset = + with(operationReference) { startOffset + textLength } + .plus( + operationReference + .nextSibling() + ?.takeIf { it.isWhiteSpace() } + ?.textLength + ?: 0, + ) + emit(startOffset, "Newline expected before operand in multiline condition", true) + if (autoCorrect) { + operationReference.upsertWhitespaceAfterMe(indentConfig.siblingIndentOf(node)) + } + } + } + + private fun ASTNode.isMultiline(): Boolean { + if (this.leftHandSide().isMultilineOperand() || + this.rightHandSide().isMultilineOperand() + ) { + return true + } + + return anyParentBinaryExpression { parent -> + parent.children().any { it.isWhiteSpaceWithNewline() } + } + } + + private fun ASTNode.leftHandSide() = children().firstOrNull { !it.isWhiteSpace() && !it.isPartOfComment() } + + private fun ASTNode.rightHandSide() = children().lastOrNull { !it.isWhiteSpace() && !it.isPartOfComment() } + + private fun ASTNode?.isMultilineOperand() = + if (this == null) { + false + } else { + leavesInClosedRange(this.firstChildLeafOrSelf(), this.lastChildLeafOrSelf()) + .any { it.isWhiteSpaceWithNewline() } + } + + private fun ASTNode.anyParentBinaryExpression(predicate: (ASTNode) -> Boolean): Boolean { + var current = this + while (current.elementType == BINARY_EXPRESSION) { + if (predicate(current)) { + return true + } + current = current.treeParent + } + return false + } + + private companion object { + val logicalOperators = TokenSet.create(OROR, ANDAND) + } +} + +public val CONDITION_WRAPPING_RULE_ID: RuleId = ConditionWrappingRule().ruleId diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IfElseWrappingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IfElseWrappingRule.kt index 04e50d8a7b..ddeb59210b 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IfElseWrappingRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IfElseWrappingRule.kt @@ -129,10 +129,7 @@ public class IfElseWrappingRule : multilineIf: Boolean, ) { if (multilineIf) { - if (node.elementType == IF && node.prevCodeLeaf()?.elementType == ELSE_KEYWORD || - node.elementType == ELSE && node.nextCodeLeaf()?.elementType == IF_KEYWORD || - node.elementType == ELSE_KEYWORD && node.nextCodeLeaf()?.elementType == IF_KEYWORD - ) { + if (node.isElseIf()) { // Allow "else if" on single line return } @@ -168,6 +165,13 @@ public class IfElseWrappingRule : } } + private fun ASTNode.isElseIf() = + when (elementType) { + IF -> prevCodeLeaf()?.elementType == ELSE_KEYWORD + ELSE, ELSE_KEYWORD -> nextCodeLeaf()?.elementType == IF_KEYWORD + else -> false + } + private fun ASTNode.findFirstNodeInBlockToBeIndented(): ASTNode? = findChildByType(BLOCK) ?.children() diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ImportOrderingRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ImportOrderingRule.kt index 65d46e2f6f..666fa46e45 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ImportOrderingRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ImportOrderingRule.kt @@ -7,6 +7,7 @@ import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.STABLE 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.isWhiteSpace import com.pinterest.ktlint.ruleset.standard.StandardRule import com.pinterest.ktlint.ruleset.standard.rules.ImportOrderingRule.Companion.ASCII_PATTERN import com.pinterest.ktlint.ruleset.standard.rules.ImportOrderingRule.Companion.IDEA_PATTERN @@ -140,20 +141,17 @@ public class ImportOrderingRule : val importTextSet = mutableSetOf() children.forEach { current -> - val isPsiWhiteSpace = current.psi is PsiWhiteSpace - - if (current.elementType == ElementType.IMPORT_DIRECTIVE || - isPsiWhiteSpace && current.textLength > 1 // also collect empty lines, that are represented as "\n\n" - ) { - if (isPsiWhiteSpace || importTextSet.add(current.text)) { + when { + current.isWhiteSpace() && current.text.count { it == '\n' } > 1 -> imports += current - } else { - emit( - current.startOffset, - "Duplicate '${current.text}' found", - true, - ) - autoCorrectDuplicateImports = true + + current.elementType == ElementType.IMPORT_DIRECTIVE -> { + if (importTextSet.add(current.text)) { + imports += current + } else { + emit(current.startOffset, "Duplicate '${current.text}' found", true) + autoCorrectDuplicateImports = true + } } } } diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRule.kt new file mode 100644 index 0000000000..c3318a5fab --- /dev/null +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRule.kt @@ -0,0 +1,82 @@ +package com.pinterest.ktlint.ruleset.standard.rules + +import com.pinterest.ktlint.rule.engine.core.api.ElementType.ANDAND +import com.pinterest.ktlint.rule.engine.core.api.ElementType.BINARY_EXPRESSION +import com.pinterest.ktlint.rule.engine.core.api.ElementType.OPERATION_REFERENCE +import com.pinterest.ktlint.rule.engine.core.api.ElementType.OROR +import com.pinterest.ktlint.rule.engine.core.api.Rule +import com.pinterest.ktlint.rule.engine.core.api.RuleId +import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint +import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.EXPERIMENTAL +import com.pinterest.ktlint.rule.engine.core.api.parent +import com.pinterest.ktlint.ruleset.standard.StandardRule +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet + +/** + * Conditions should not use a both '&&' and '||' operators between operators at the same level. By using parenthesis the expression is to + * be clarified. + */ +@SinceKtlint("1.1.0", EXPERIMENTAL) +public class MixedConditionOperatorsRule : + StandardRule("condition-wrapping"), + Rule.Experimental { + override fun beforeVisitChildNodes( + node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, + ) { + node + .takeIf { it.isLogicalBinaryExpression() } + ?.takeIf { it.isPartOfExpressionUsingDifferentLogicalOperators() } + ?.let { + visitLogicalExpression(it, emit) + } + } + + private fun ASTNode.isLogicalBinaryExpression() = + takeIf { elementType == BINARY_EXPRESSION } + ?.findChildByType(OPERATION_REFERENCE) + ?.let { it.firstChildNode.elementType in logicalOperators } + ?: false + + private fun visitLogicalExpression( + node: ASTNode, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, + ) { + node + .parent { it.elementType == BINARY_EXPRESSION && it.treeParent.elementType != BINARY_EXPRESSION } + ?.let { rootBinaryExpression -> + emit( + rootBinaryExpression.startOffset, + "A condition with mixed usage of '&&' and '||' is hard to read. Use parenthesis to clarify the (sub)condition.", + false, + ) + } + } + + private fun ASTNode.anyParentBinaryExpression(predicate: (ASTNode) -> Boolean): Boolean { + var current = this + while (current.elementType == BINARY_EXPRESSION) { + if (predicate(current)) { + return true + } + current = current.treeParent + } + return false + } + + private fun ASTNode.isPartOfExpressionUsingDifferentLogicalOperators(): Boolean { + val operatorStartNode = findOperatorElementTypeOrNull() ?: return false + + return anyParentBinaryExpression { it.findOperatorElementTypeOrNull() != operatorStartNode } + } + + private fun ASTNode.findOperatorElementTypeOrNull() = findChildByType(OPERATION_REFERENCE)?.firstChildNode?.elementType + + private companion object { + val logicalOperators = TokenSet.create(OROR, ANDAND) + } +} + +public val MIXED_CONDITION_OPERATORS_RULE_ID: RuleId = MixedConditionOperatorsRule().ruleId diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoEmptyFirstLineInMethodBlockRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoEmptyFirstLineInMethodBlockRule.kt index e55ea2c08b..b7f3ffb6ce 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoEmptyFirstLineInMethodBlockRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoEmptyFirstLineInMethodBlockRule.kt @@ -1,17 +1,17 @@ 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.CLASS_BODY import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN +import com.pinterest.ktlint.rule.engine.core.api.ElementType.LBRACE import com.pinterest.ktlint.rule.engine.core.api.RuleId import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.EXPERIMENTAL import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.STABLE import com.pinterest.ktlint.rule.engine.core.api.isPartOf +import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline import com.pinterest.ktlint.rule.engine.core.api.prevLeaf import com.pinterest.ktlint.ruleset.standard.StandardRule import org.jetbrains.kotlin.com.intellij.lang.ASTNode -import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement @SinceKtlint("0.34", EXPERIMENTAL) @@ -22,11 +22,14 @@ public class NoEmptyFirstLineInMethodBlockRule : StandardRule("no-empty-first-li autoCorrect: Boolean, emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, ) { - if (node is PsiWhiteSpace && node.textContains('\n') && - node.prevLeaf()?.elementType == ElementType.LBRACE && node.isPartOf(FUN) && - node.treeParent.elementType != CLASS_BODY // fun fn() = object : Builder {\n\n fun stuff() = Unit } + if (node.isWhiteSpaceWithNewline() && + node.prevLeaf()?.elementType == LBRACE && + node.isPartOf(FUN) && + // Allow: + // fun fn() = object : Builder {\n\n fun stuff() = Unit } + node.treeParent.elementType != CLASS_BODY ) { - val split = node.getText().split("\n") + val split = node.text.split("\n") if (split.size > 2) { emit( node.startOffset + 1, diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSemicolonsRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSemicolonsRule.kt index c80ab10159..007ea1423b 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSemicolonsRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoSemicolonsRule.kt @@ -82,7 +82,8 @@ public class NoSemicolonsRule : psi.getStrictParentOfType() == null && psi.getStrictParentOfType() == null }.let { nextLeaf -> - nextLeaf == null || // \s+ and then eof + nextLeaf == null || + // \s+ and then eof (textContains('\n') && nextLeaf.elementType != KtTokens.LBRACE) } } diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoUnusedImportsRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoUnusedImportsRule.kt index af73f825f8..73a9665fc6 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoUnusedImportsRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/NoUnusedImportsRule.kt @@ -148,7 +148,8 @@ public class NoUnusedImportsRule : StandardRule("no-unused-imports") { if (autoCorrect) { importDirective.delete() } - } else if (name != null && (!ref.map { it.text }.contains(name) || !isAValidImport(importPath)) && + } else if (name != null && + (!ref.map { it.text }.contains(name) || !isAValidImport(importPath)) && !OPERATOR_SET.contains(name) && !name.isComponentN() && !importPath.ignoreProvideDelegate() diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt index c666a8c563..b3f04be4fd 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundCurlyRule.kt @@ -28,6 +28,8 @@ import com.pinterest.ktlint.rule.engine.core.api.isLeaf import com.pinterest.ktlint.rule.engine.core.api.isPartOfComment import com.pinterest.ktlint.rule.engine.core.api.isPartOfString import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace +import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline +import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithoutNewline import com.pinterest.ktlint.rule.engine.core.api.leavesIncludingSelf import com.pinterest.ktlint.rule.engine.core.api.nextLeaf import com.pinterest.ktlint.rule.engine.core.api.prevLeaf @@ -96,45 +98,46 @@ public class SpacingAroundCurlyRule : prevLeaf.node.treeParent.removeChild(prevLeaf.node) } } - if (prevLeaf is PsiWhiteSpace && prevLeaf.textContains('\n') && - ( + prevLeaf + ?.takeIf { it.isWhiteSpaceWithNewline() } + ?.takeIf { prevLeaf.prevLeaf()?.let { it.elementType == RPAR || KtTokens.KEYWORDS.contains(it.elementType) } == true || node.treeParent.elementType == CLASS_BODY || + // allow newline for lambda return type (prevLeaf.treeParent.elementType == FUN && prevLeaf.treeNext.elementType != LAMBDA_EXPRESSION) - ) // allow newline for lambda return type - ) { - emit(node.startOffset, "Unexpected newline before \"${node.text}\"", true) - if (autoCorrect) { - if (prevLeaf.isPrecededByEolComment()) { - // All consecutive whitespaces and comments preceding the curly have to be moved after the curly brace - prevLeaf - .leavesIncludingSelf(forward = false) - .takeWhile { it.isWhiteSpace() || it.isPartOfComment() } - .toList() - .reversed() - .takeIf { it.isNotEmpty() } - ?.let { leavesToMoveAfterCurly -> - node.treeParent.addChildren( - leavesToMoveAfterCurly.first(), - leavesToMoveAfterCurly.last(), - node.treeNext, - ) - } + }?.run { + emit(node.startOffset, "Unexpected newline before \"${node.text}\"", true) + if (autoCorrect) { + if (isPrecededByEolComment()) { + // All consecutive whitespaces and comments preceding the curly have to be moved after the curly brace + leavesIncludingSelf(forward = false) + .takeWhile { it.isWhiteSpace() || it.isPartOfComment() } + .toList() + .reversed() + .takeIf { it.isNotEmpty() } + ?.let { leavesToMoveAfterCurly -> + node.treeParent.addChildren( + leavesToMoveAfterCurly.first(), + leavesToMoveAfterCurly.last(), + node.treeNext, + ) + } + } + (this as LeafPsiElement).rawReplaceWithText(" ") } - (prevLeaf as LeafPsiElement).rawReplaceWithText(" ") } - } } else if (node.elementType == RBRACE) { spacingBefore = prevLeaf is PsiWhiteSpace || prevLeaf?.elementType == LBRACE spacingAfter = nextLeaf == null || nextLeaf is PsiWhiteSpace || shouldNotToBeSeparatedBySpace(nextLeaf) - if (nextLeaf is PsiWhiteSpace && !nextLeaf.textContains('\n') && - shouldNotToBeSeparatedBySpace(nextLeaf.nextLeaf()) - ) { - emit(node.startOffset, "Unexpected space after \"${node.text}\"", true) - if (autoCorrect) { - nextLeaf.node.treeParent.removeChild(nextLeaf.node) + nextLeaf + .takeIf { it.isWhiteSpaceWithoutNewline() } + ?.takeIf { shouldNotToBeSeparatedBySpace(it.nextLeaf()) } + ?.let { leaf -> + emit(node.startOffset, "Unexpected space after \"${node.text}\"", true) + if (autoCorrect) { + leaf.treeParent.removeChild(leaf) + } } - } } else { return } diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundOperatorsRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundOperatorsRule.kt index 5dc088adfc..feacfc439d 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundOperatorsRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundOperatorsRule.kt @@ -54,9 +54,9 @@ public class SpacingAroundOperatorsRule : StandardRule("op-spacing") { emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit, ) { if (tokenSet.contains(node.elementType) && - !node.isPartOf(KtPrefixExpression::class) && // not unary - !(node.elementType == MUL && node.treeParent.elementType == VALUE_ARGUMENT) && // fn(*array) - !node.isPartOf(KtImportDirective::class) // import * + node.isNotUnaryOperator() && + isNotSpreadOperator(node) && + isNotImport(node) ) { if ((node.elementType == LT || node.elementType == GT || node.elementType == MUL) && node.treeParent.elementType != OPERATION_REFERENCE @@ -92,6 +92,16 @@ public class SpacingAroundOperatorsRule : StandardRule("op-spacing") { } } } + + private fun ASTNode.isNotUnaryOperator() = !isPartOf(KtPrefixExpression::class) + + private fun isNotSpreadOperator(node: ASTNode) = + // fn(*array) + !(node.elementType == MUL && node.treeParent.elementType == VALUE_ARGUMENT) + + private fun isNotImport(node: ASTNode) = + // import * + !node.isPartOf(KtImportDirective::class) } public val SPACING_AROUND_OPERATORS_RULE_ID: RuleId = SpacingAroundOperatorsRule().ruleId diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundParensRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundParensRule.kt index 1b4878df0b..194d02cc25 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundParensRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/SpacingAroundParensRule.kt @@ -14,6 +14,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER_LIS import com.pinterest.ktlint.rule.engine.core.api.RuleId import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.STABLE +import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithoutNewline import com.pinterest.ktlint.rule.engine.core.api.nextLeaf import com.pinterest.ktlint.rule.engine.core.api.prevLeaf import com.pinterest.ktlint.ruleset.standard.StandardRule @@ -37,7 +38,8 @@ public class SpacingAroundParensRule : StandardRule("paren-spacing") { val nextLeaf = node.nextLeaf() val spacingBefore = if (node.elementType == LPAR) { - prevLeaf is PsiWhiteSpace && !prevLeaf.textContains('\n') && + prevLeaf is PsiWhiteSpace && + !prevLeaf.textContains('\n') && ( prevLeaf.prevLeaf()?.elementType == IDENTIFIER && // val foo: @Composable () -> Unit @@ -51,8 +53,7 @@ public class SpacingAroundParensRule : StandardRule("paren-spacing") { node.treeParent?.elementType == VALUE_ARGUMENT_LIST ) } else { - prevLeaf is PsiWhiteSpace && !prevLeaf.textContains('\n') && - prevLeaf.prevLeaf()?.elementType != LPAR + prevLeaf.isWhiteSpaceWithoutNewline() && prevLeaf?.prevLeaf()?.elementType != LPAR } val spacingAfter = if (node.elementType == LPAR) { @@ -60,8 +61,7 @@ public class SpacingAroundParensRule : StandardRule("paren-spacing") { (!nextLeaf.textContains('\n') || nextLeaf.nextLeaf()?.elementType == RPAR) && !nextLeaf.isNextLeafAComment() } else { - nextLeaf is PsiWhiteSpace && !nextLeaf.textContains('\n') && - nextLeaf.nextLeaf()?.elementType == RPAR + nextLeaf.isWhiteSpaceWithoutNewline() && nextLeaf?.nextLeaf()?.elementType == RPAR } when { spacingBefore && spacingAfter -> { diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRuleTest.kt new file mode 100644 index 0000000000..bf453d6d99 --- /dev/null +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ConditionWrappingRuleTest.kt @@ -0,0 +1,252 @@ +package com.pinterest.ktlint.ruleset.standard.rules + +import com.pinterest.ktlint.ruleset.standard.StandardRuleSetProvider +import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRuleBuilder +import com.pinterest.ktlint.test.LintViolation +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +class ConditionWrappingRuleTest { + private val conditionWrappingRuleAssertThat = + assertThatRuleBuilder { ConditionWrappingRule() } + .addAdditionalRuleProvider { IndentationRule() } + .addRequiredRuleProviderDependenciesFrom(StandardRuleSetProvider()) + .assertThat() + + @Test + fun `Given all operands on the same line`() { + val code = + """ + val foo = bar || baz + """.trimIndent() + conditionWrappingRuleAssertThat(code).hasNoLintViolations() + } + + @Nested + inner class `Given a simple condition with a multiline operand in the left hand side` { + @Test + fun `Given the right hand side operand is preceded by a space`() { + val code = + """ + val foo = + multiLineOperand( + "bar" + ) || baz + """.trimIndent() + val formattedCode = + """ + val foo = + multiLineOperand( + "bar" + ) || + baz + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(4, 10, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given the right hand side operand is not preceded by a whitespace`() { + val code = + """ + val foo = + multiLineOperand( + "bar" + ) ||baz + """.trimIndent() + val formattedCode = + """ + val foo = + multiLineOperand( + "bar" + ) || + baz + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(4, 9, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given the right hand side operand is preceded by a newline`() { + val code = + """ + val foo = + multiLineOperand( + "bar" + ) || + baz + """.trimIndent() + conditionWrappingRuleAssertThat(code).hasNoLintViolations() + } + } + + @Nested + inner class `Given a simple condition with a multiline operand in the right hand side` { + @Test + fun `Given the operand is preceded by a space`() { + val code = + """ + val foo = + bar || multiLineOperand( + "baz" + ) + """.trimIndent() + val formattedCode = + """ + val foo = + bar || + multiLineOperand( + "baz" + ) + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(2, 12, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given the operand is not preceded by a whitespace`() { + val code = + """ + val foo = + bar ||multiLineOperand( + "baz" + ) + """.trimIndent() + val formattedCode = + """ + val foo = + bar || + multiLineOperand( + "baz" + ) + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(2, 11, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given the operand is preceded by a newline`() { + val code = + """ + val foo = + bar || + multiLineOperand( + "baz" + ) + """.trimIndent() + conditionWrappingRuleAssertThat(code).hasNoLintViolations() + } + } + + @Test + fun `Given not all operands are on the same line`() { + val code = + """ + val foo = + bar1 || bar2 || + baz1 || baz2 || baz3 + """.trimIndent() + val formattedCode = + """ + val foo = + bar1 || + bar2 || + baz1 || + baz2 || + baz3 + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolations( + LintViolation(2, 13, "Newline expected before operand in multiline condition"), + LintViolation(3, 17, "Newline expected before operand in multiline condition"), + LintViolation(3, 25, "Newline expected before operand in multiline condition"), + ).isFormattedAs(formattedCode) + } + + @Nested + inner class `Given a parenthesized logical expression for which all inner operands are on the same line` { + @Test + fun `Given first operand is parenthesized`() { + val code = + """ + val foo = + (baz1 && baz2) || bar1 || + bar2 + """.trimIndent() + val formattedCode = + """ + val foo = + (baz1 && baz2) || + bar1 || + bar2 + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(2, 23, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given second operand on first line is parenthesized`() { + val code = + """ + val foo = + bar1 || (baz1 && baz2) || + bar2 + """.trimIndent() + val formattedCode = + """ + val foo = + bar1 || + (baz1 && baz2) || + bar2 + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(2, 13, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given second operand on second line is parenthesized`() { + val code = + """ + val foo = + bar1 || + (baz1 && baz2) || bar2 + """.trimIndent() + val formattedCode = + """ + val foo = + bar1 || + (baz1 && baz2) || + bar2 + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(3, 27, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + + @Test + fun `Given last operand on second line is parenthesized`() { + val code = + """ + val foo = + bar1 || bar2 || + (baz1 && baz2) + """.trimIndent() + val formattedCode = + """ + val foo = + bar1 || + bar2 || + (baz1 && baz2) + """.trimIndent() + conditionWrappingRuleAssertThat(code) + .hasLintViolation(2, 13, "Newline expected before operand in multiline condition") + .isFormattedAs(formattedCode) + } + } +} diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRuleTest.kt new file mode 100644 index 0000000000..1011e28814 --- /dev/null +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MixedConditionOperatorsRuleTest.kt @@ -0,0 +1,52 @@ +package com.pinterest.ktlint.ruleset.standard.rules + +import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule +import org.junit.jupiter.api.Test + +class MixedConditionOperatorsRuleTest { + private val mixedConditionOperatorsRuleAssertThat = assertThatRule { MixedConditionOperatorsRule() } + + @Test + fun `Given a single line condition with mixed logical operators in the same expression`() { + val code = + """ + val foo = bar1 && bar2 || bar3 + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + mixedConditionOperatorsRuleAssertThat(code) + .hasLintViolationWithoutAutoCorrect(1, 11, "A condition with mixed usage of '&&' and '||' is hard to read. Use parenthesis to clarify the (sub)condition.") + } + + @Test + fun `Given a multiline condition with mixed logical operators in the same expression`() { + val code = + """ + val foo = bar1 && + bar2 || + bar3 + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + mixedConditionOperatorsRuleAssertThat(code) + .hasLintViolationWithoutAutoCorrect(1, 11, "A condition with mixed usage of '&&' and '||' is hard to read. Use parenthesis to clarify the (sub)condition.") + } + + @Test + fun `Given a condition same logical operators in the expression but a different operator in a subexpression`() { + val code = + """ + val foo = bar1 && (bar2 || bar3) && bar4 + """.trimIndent() + mixedConditionOperatorsRuleAssertThat(code).hasNoLintViolations() + } + + @Test + fun `Given a condition same logical operators in the expression and different operators in a subexpression`() { + val code = + """ + val foo = bar1 && (bar2 || bar3 && bar4) && bar5 + """.trimIndent() + @Suppress("ktlint:standard:argument-list-wrapping", "ktlint:standard:max-line-length") + mixedConditionOperatorsRuleAssertThat(code) + .hasLintViolationWithoutAutoCorrect(1, 20, "A condition with mixed usage of '&&' and '||' is hard to read. Use parenthesis to clarify the (sub)condition.") + } +}