Skip to content

Commit

Permalink
Add rule blank-line-between-when-conditions (pinterest#2564)
Browse files Browse the repository at this point in the history
  • Loading branch information
paul-dingemans authored Feb 27, 2024
1 parent e12a648 commit 60b6b8f
Show file tree
Hide file tree
Showing 40 changed files with 597 additions and 4 deletions.
81 changes: 81 additions & 0 deletions documentation/snapshot/docs/rules/experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,87 @@ Wraps binary expression at the operator reference whenever the binary expression

Rule id: `binary-expression-wrapping` (`standard` rule set)

## Blank lines between when-conditions

Consistently add or remove blank lines between when-conditions in a when-statement. A blank line is only added between when-conditions if the when-statement contains at lease one multiline when-condition. If a when-statement only contains single line when-conditions, then the blank lines between the when-conditions are removed.

!!! note
Ktlint uses `.editorconfig` property `ij_kotlin_line_break_after_multiline_when_entry` but applies it also on single line entries to increase consistency.

=== "[:material-heart:](#) Ktlint"

```kotlin
val foo1 =
when (bar) {
BAR1 -> "bar1"
BAR2 -> "bar2"
else -> null
}

// ij_kotlin_line_break_after_multiline_when_entry = true
val foo2 =
when (bar) {
BAR1 -> "bar1"

BAR2 -> {
"bar2"
}

else -> null
}

// ij_kotlin_line_break_after_multiline_when_entry = false
val foo2 =
when (bar) {
BAR1 -> "bar1"
BAR2 -> {
"bar2"
}
else -> null
}
```
=== "[:material-heart-off-outline:](#) Disallowed"

```kotlin
// ij_kotlin_line_break_after_multiline_when_entry = true | false (no blank lines in simple when-statement)
val foo1 =
when (bar) {
BAR1 -> "bar1"

BAR2 -> "bar2"

else -> null
}

// ij_kotlin_line_break_after_multiline_when_entry = true (missing newline after BAR1)
val foo2 =
when (bar) {
BAR1 -> "bar1"
BAR2 -> {
"bar2"
}

else -> null
}

// ij_kotlin_line_break_after_multiline_when_entry = false (unexpected newline after BAR2)
val foo2 =
when (bar) {
BAR1 -> "bar1"
BAR2 -> {
"bar2"
}

else -> null
}
```

| Configuration setting | ktlint_official | intellij_idea | android_studio |
|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:---------------:|:-------------:|:--------------:|
| `ij_kotlin_line_break_after_multiline_when_entry`<br/><i>Despite its name, forces a blank line between single line and multiline when-entries when at least one multiline when-entry is found in the when-statement.</i> | `true` | `true` | `true` |

Rule id: `blank-lines-between-when-conditions` (`standard` rule set)

## Chain method continuation

In a multiline method chain, the chain operators (`.` or `?.`) have to be aligned with each other.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class FormatReporter(
countAutoCorrectPossibleOrDone.putIfAbsent(file, 0)
countAutoCorrectPossibleOrDone.replace(file, countAutoCorrectPossibleOrDone.getOrDefault(file, 0) + 1)
}

else -> {
countCanNotBeAutoCorrected.putIfAbsent(file, 0)
countCanNotBeAutoCorrected.replace(file, countCanNotBeAutoCorrected.getOrDefault(file, 0) + 1)
Expand All @@ -46,18 +47,21 @@ public class FormatReporter(
} else {
"Format required (1 violation needs manual fixing)"
}

canNotBeAutocorrected > 1 ->
if (format) {
"Format not completed ($canNotBeAutocorrected violations need manual fixing)"
} else {
"Format required ($canNotBeAutocorrected violations need manual fixing)"
}

countAutoCorrectPossibleOrDone.getOrDefault(file, 0) > 0 ->
if (format) {
"Format completed (all violations have been fixed)"
} else {
"Format required (all violations can be autocorrected)"
}

else ->
"Format not needed (no violations found)"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,10 @@ private fun Path.findCommonParentDir(path: Path): Path =
when {
path.startsWith(this) ->
this

startsWith(path) ->
path

else ->
this@findCommonParentDir.findCommonParentDir(path.parent)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ internal class KtlintCommandLine :
}.also { formattedFileContent ->
when {
code.isStdIn -> print(formattedFileContent)

code.content != formattedFileContent ->
code
.filePath
Expand Down Expand Up @@ -616,6 +617,7 @@ internal class KtlintCommandLine :
detail = "Not a valid Kotlin file (${e.message?.lowercase(Locale.getDefault())})",
status = KOTLIN_PARSE_EXCEPTION,
)

is KtLintRuleException -> {
logger.debug(e) { "Internal Error (${e.ruleId}) in ${code.fileNameOrStdin()} at position '${e.line}:${e.col}" }
KtlintCliError(
Expand All @@ -630,6 +632,7 @@ internal class KtlintCommandLine :
status = KTLINT_RULE_ENGINE_EXCEPTION,
)
}

else -> throw e
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ class CommandLineTestRunner(
key.equals(PATH, ignoreCase = true)
} ?: PATH
}

else -> PATH
}
environment[pathKey] = "$JAVA_HOME_BIN_DIR${File.pathSeparator}${OsEnvironment()[PATH]}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ public class IndentConfig(
require(indent.matches(TABS_AND_SPACES))
return when (indentStyle) {
SPACE -> indent.replaceTabWithSpaces()

TAB -> {
"\t".repeat(indentLevelFrom(indent))
// Silently swallow spaces if not enough spaces present to convert to a tab
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,17 @@ private fun KtlintSuppressionAtOffset.offsetFromStartOf(code: String): Int {
col == 1 && codeLine.isEmpty() -> {
startOffsetOfLineContainingLintError
}

col <= codeLine.length -> {
startOffsetOfLineContainingLintError + (col - 1)
}

col == codeLine.length + 1 -> {
// Offset of suppression is set at EOL of the line. This is visually correct for the reader. But the newline character was stripped
// from the line because the lines were split using that character.
startOffsetOfLineContainingLintError + col
}

else -> {
throw KtlintSuppressionOutOfBoundsException(this)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ private class SegmentTree(
): Int =
when {
l > r -> -1

else -> {
val i = l + (r - l) / 2
val s = segments[i]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ internal object SuppressionLocatorBuilder {
.let { suppressedRuleIds ->
when {
suppressedRuleIds.isEmpty() -> null

suppressedRuleIds.contains(ALL_KTLINT_RULES_SUPPRESSION_ID) ->
SuppressionHint(
IntRange(startOffset, endOffset - 1),
Expand All @@ -244,6 +245,7 @@ internal object SuppressionLocatorBuilder {
// Disable all rules
ALL_KTLINT_RULES_SUPPRESSION_ID
}

argumentExpressionText.startsWith("ktlint:") -> {
// Disable specific rule. For backwards compatibility prefix rules without rule set id with the "standard" rule set
// id. Note that the KtlintSuppressionRule will emit a lint violation on the id. So this fix is only applicable for
Expand All @@ -252,6 +254,7 @@ internal object SuppressionLocatorBuilder {
.removePrefix("ktlint:")
.let { RuleId.prefixWithStandardRuleSetIdWhenMissing(it) }
}

else -> {
// Disable specific rule if the annotation value is mapped to a specific rule
annotationValueToRuleMapping[argumentExpressionText]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,13 @@ private class RuleExecutionFilter(
when {
rule is Rule.Experimental && rule is Rule.OfficialCodeStyle ->
isExperimentalEnabled(rule) && isOfficialCodeStyleEnabled(rule)

rule is Rule.Experimental ->
isExperimentalEnabled(rule)

rule is Rule.OfficialCodeStyle ->
isOfficialCodeStyleEnabled(rule)

else ->
isRuleSetEnabled(rule)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ private class AutoCorrectErrorRule :
(node as LeafElement).rawReplaceWithText(STRING_VALUE_AFTER_AUTOCORRECT)
}
}

STRING_VALUE_NOT_TO_BE_CORRECTED ->
emit(node.startOffset, ERROR_MESSAGE_CAN_NOT_BE_AUTOCORRECTED, false)
}
Expand Down
15 changes: 15 additions & 0 deletions ktlint-ruleset-standard/api/ktlint-ruleset-standard.api
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/BlankLineBeforeDe
public static final fun getBLANK_LINE_BEFORE_DECLARATION_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}

public final class com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditions : com/pinterest/ktlint/ruleset/standard/StandardRule {
public static final field Companion Lcom/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditions$Companion;
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/BlankLineBetweenWhenConditions$Companion {
public final fun getLINE_BREAK_AFTER_WHEN_CONDITION_PROPERTY ()Lcom/pinterest/ktlint/rule/engine/core/api/editorconfig/EditorConfigProperty;
}

public final class com/pinterest/ktlint/ruleset/standard/rules/BlankLineBetweenWhenConditionsKt {
public static final fun getBLANK_LINE_BETWEEN_WHEN_CONDITIONS_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}

public final class com/pinterest/ktlint/ruleset/standard/rules/BlockCommentInitialStarAlignmentRule : 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.pinterest.ktlint.ruleset.standard.rules.ArgumentListWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.BackingPropertyNamingRule
import com.pinterest.ktlint.ruleset.standard.rules.BinaryExpressionWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.BlankLineBeforeDeclarationRule
import com.pinterest.ktlint.ruleset.standard.rules.BlankLineBetweenWhenConditions
import com.pinterest.ktlint.ruleset.standard.rules.BlockCommentInitialStarAlignmentRule
import com.pinterest.ktlint.ruleset.standard.rules.ChainMethodContinuationRule
import com.pinterest.ktlint.ruleset.standard.rules.ChainWrappingRule
Expand Down Expand Up @@ -109,6 +110,7 @@ public class StandardRuleSetProvider : RuleSetProviderV3(RuleSetId.STANDARD) {
RuleProvider { BackingPropertyNamingRule() },
RuleProvider { BinaryExpressionWrappingRule() },
RuleProvider { BlankLineBeforeDeclarationRule() },
RuleProvider { BlankLineBetweenWhenConditions() },
RuleProvider { BlockCommentInitialStarAlignmentRule() },
RuleProvider { ChainMethodContinuationRule() },
RuleProvider { ChainWrappingRule() },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,21 @@ public class AnnotationRule :
visitAnnotationList(node, emit, autoCorrect)
visitFileAnnotationList(node, emit, autoCorrect)
}

ANNOTATED_EXPRESSION, MODIFIER_LIST -> {
visitAnnotationList(node, emit, autoCorrect)
}

ANNOTATION -> {
// Annotation array
// @[...]
visitAnnotation(node, emit, autoCorrect)
}

ANNOTATION_ENTRY -> {
visitAnnotationEntry(node, emit, autoCorrect)
}

TYPE_ARGUMENT_LIST -> {
visitTypeArgumentList(node, emit, autoCorrect)
}
Expand All @@ -147,8 +151,10 @@ public class AnnotationRule :
when {
node.elementType == ANNOTATED_EXPRESSION ->
indentConfig.siblingIndentOf(node)

node.hasAnnotationBeforeConstructor() ->
indentConfig.siblingIndentOf(node.treeParent)

else ->
indentConfig.parentIndentOf(node)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ public class ArgumentListWrappingRule :
// 2
// )
child.treeParent.hasTypeArgumentListInFront() -> -1

// IDEA quirk:
// foo
// .bar = Baz(
Expand All @@ -161,7 +160,6 @@ public class ArgumentListWrappingRule :
// 2
// )
child.treeParent.isPartOfDotQualifiedAssignmentExpression() -> -1

else -> 0
}.let {
if (child.treeParent.isOnSameLineAsControlFlowKeyword()) {
Expand Down Expand Up @@ -198,6 +196,7 @@ public class ArgumentListWrappingRule :
}
}
}

VALUE_ARGUMENT,
RPAR,
-> {
Expand Down
Loading

0 comments on commit 60b6b8f

Please sign in to comment.