Skip to content

Commit

Permalink
Add new rule when-entry-bracing (pinterest#2829)
Browse files Browse the repository at this point in the history
Enforce consistent usage of braces in when entries. In case a when statement contains at least one when entry which uses braces around the body of the entry, then all when entries should use braces.

Braces are helpful for following reasons:
- Bodies of the when-conditions are all aligned at same column position
- Closing braces helps in separating the when-conditions

This rule is not incorporated in the Kotlin Coding conventions, nor in the Android Kotlin Styleguide. It is based on similar behavior in enforcing consistent use of braces in if-else statements. As of that the rule is only enabled automatically for code style `ktlint_official`. It can be enabled explicitly for other code styles.

Closes pinterest#2557
  • Loading branch information
paul-dingemans authored Oct 14, 2024
1 parent 4a7e9d7 commit 50e44a7
Show file tree
Hide file tree
Showing 44 changed files with 773 additions and 157 deletions.
73 changes: 73 additions & 0 deletions documentation/snapshot/docs/rules/experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,76 @@ Suppress or disable rule (1)
ktlint_standard_square-brackets-spacing = disabled
```
## When-entry bracing
Enforce consistent usages of braces inside the when-statement. All when-entries in the when-statement should use braces around their bodies in case at least one when-entry has a multiline body, or when the body is surrounded by braces.
Braces are helpful for following reasons:
- Bodies of the when-conditions are all aligned at same column position
- Closing braces helps in separating the when-conditions
This rule is not incorporated in the Kotlin Coding conventions, nor in the Android Kotlin Styleguide. It is based on similar behavior in enforcing consistent use of braces in if-else statements. As of that the rule is only enabled automatically for code style `ktlint_official`. It can be enabled explicitly for other code styles.
=== "[:material-heart:](#) Ktlint"
```kotlin
val foo1 =
when (bar) {
BAR1 -> "bar1"
BAR2 -> "bar2"
else -> null
}
val foo2 =
when (bar) {
BAR1 -> {
"bar1"
}
BAR2 -> {
"bar2"
}
else -> {
null
}
}
```
=== "[:material-heart-off-outline:](#) Disallowed"
```kotlin
val foo3 =
when (bar) {
BAR1 -> "bar1"
BAR2 -> {
"bar2"
}
else -> null
}
val foo4 =
when (bar) {
BAR1 -> "bar1"
BAR2 ->
"bar2"
else -> null
}
```
Rule id: `standard:when-entry-spacing`
Suppress or disable rule (1)
{ .annotate }
1. Suppress rule in code with annotation below:
```kotlin
@Suppress("ktlint:standard:when-entry-spacing")
```
Enable rule via `.editorconfig`
```editorconfig
ktlint_standard_when-entry-spacing = enabled
```
Disable rule via `.editorconfig`
```editorconfig
ktlint_standard_when-entry-spacing = disabled
```
Original file line number Diff line number Diff line change
Expand Up @@ -41,29 +41,33 @@ public class FormatReporter(
val canNotBeAutocorrected = countCanNotBeAutoCorrected.getOrDefault(file, 0)
val result =
when {
canNotBeAutocorrected == 1 ->
canNotBeAutocorrected == 1 -> {
if (format) {
"Format not completed (1 violation needs manual fixing)"
} else {
"Format required (1 violation needs manual fixing)"
}
}

canNotBeAutocorrected > 1 ->
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 ->
countAutoCorrectPossibleOrDone.getOrDefault(file, 0) > 0 -> {
if (format) {
"Format completed (all violations have been fixed)"
} else {
"Format required (all violations can be autocorrected)"
}
}

else ->
else -> {
"Format not needed (no violations found)"
}
}
out.println(
"${colorFileName(file)}${":".colored()} $result",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,9 @@ internal fun FileSystem.fileSequence(

private fun Path.findCommonParentDir(path: Path): Path =
when {
path.startsWith(this) ->
this

startsWith(path) ->
path

else ->
this@findCommonParentDir.findCommonParentDir(path.parent)
path.startsWith(this) -> this
startsWith(path) -> path
else -> this@findCommonParentDir.findCommonParentDir(path.parent)
}

private fun FileSystem.expand(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,13 +526,16 @@ internal class KtlintCommandLine : CliktCommand(name = "ktlint") {
}
}
when {
code.isStdIn -> print(formattedFileContent)
code.isStdIn -> {
print(formattedFileContent)
}

code.content != formattedFileContent ->
code.content != formattedFileContent -> {
code
.filePath
?.toFile()
?.writeText(formattedFileContent, charset("UTF-8"))
}
}
}
} catch (e: Exception) {
Expand Down Expand Up @@ -637,14 +640,15 @@ internal class KtlintCommandLine : CliktCommand(name = "ktlint") {
private fun Exception.toKtlintCliError(code: Code): KtlintCliError =
this.let { e ->
when (e) {
is KtLintParseException ->
is KtLintParseException -> {
KtlintCliError(
line = e.line,
col = e.col,
ruleId = "",
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}" }
Expand All @@ -661,7 +665,9 @@ internal class KtlintCommandLine : CliktCommand(name = "ktlint") {
)
}

else -> throw e
else -> {
throw e
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ class CommandLineTestRunner(
arrayOf(comSpec, "/C")
}

else -> arrayOf("/bin/sh", "-c")
else -> {
arrayOf("/bin/sh", "-c")
}
}

private fun ktlintCommand(arguments: List<String>): String =
Expand Down Expand Up @@ -189,7 +191,9 @@ class CommandLineTestRunner(
} ?: PATH
}

else -> 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 @@ -111,7 +111,9 @@ public class IndentConfig(
val indent = getTextAfterLastNewLine(text)
require(indent.matches(TABS_AND_SPACES))
return when (indentStyle) {
SPACE -> indent.replaceTabWithSpaces()
SPACE -> {
indent.replaceTabWithSpaces()
}

TAB -> {
"\t".repeat(indentLevelFrom(indent))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@ public class EditorConfig(
*/
public operator fun <T> get(editorConfigProperty: EditorConfigProperty<T>): T {
when {
editorConfigProperty.deprecationError != null ->
editorConfigProperty.deprecationError != null -> {
throw DeprecatedEditorConfigPropertyException(
"Property '${editorConfigProperty.name}' is disallowed: ${editorConfigProperty.deprecationError}",
)
}

editorConfigProperty.deprecationWarning != null ->
editorConfigProperty.deprecationWarning != null -> {
LOGGER.warn { "Property '${editorConfigProperty.name}' is deprecated: ${editorConfigProperty.deprecationWarning}" }
}
}
val property =
properties.getOrElse(editorConfigProperty.name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ public val MAX_LINE_LENGTH_PROPERTY: EditorConfigProperty<Int> =
* Internally, Ktlint uses integer 'Int.MAX_VALUE' to indicate that the max line length has to be ignored as this is easier
* in comparisons to check whether the maximum length of a line is exceeded.
*/
property.sourceValue == MAX_LINE_LENGTH_PROPERTY_OFF_EDITOR_CONFIG -> MAX_LINE_LENGTH_PROPERTY_OFF
property.sourceValue == MAX_LINE_LENGTH_PROPERTY_OFF_EDITOR_CONFIG -> {
MAX_LINE_LENGTH_PROPERTY_OFF
}

else ->
else -> {
PropertyType
.max_line_length
.parse(property.sourceValue)
Expand All @@ -55,6 +57,7 @@ public val MAX_LINE_LENGTH_PROPERTY: EditorConfigProperty<Int> =
it.parsed
}
}
}
}
},
propertyWriter = { property ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ public fun KtLintRuleEngine.insertSuppression(

private fun ASTNode.findLeafElementAt(suppression: KtlintSuppression): ASTNode =
when (suppression) {
is KtlintSuppressionForFile -> this
is KtlintSuppressionForFile -> {
this
}

is KtlintSuppressionAtOffset ->
is KtlintSuppressionAtOffset -> {
findLeafElementAt(suppression.offsetFromStartOf(text))
?.let {
if (it.isWhiteSpace()) {
Expand All @@ -48,6 +50,7 @@ private fun ASTNode.findLeafElementAt(suppression: KtlintSuppression): ASTNode =
}
}
?: throw KtlintSuppressionNoElementFoundException(suppression)
}
}

private fun KtlintSuppressionAtOffset.offsetFromStartOf(code: String): Int {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ internal class CodeFormatter(
var codeContent = formattedCode(lineSeparator)
val errors = mutableSetOf<Pair<LintError, Boolean>>()
var formatRunCount = 0
var mutated: Boolean = false
var mutated = false
do {
val newErrors = format(autocorrectHandler, code)
errors.addAll(newErrors)
Expand Down Expand Up @@ -168,10 +168,13 @@ internal class CodeFormatter(
when {
eolEditorConfigProperty == PropertyType.EndOfLineValue.crlf ||
eolEditorConfigProperty != PropertyType.EndOfLineValue.lf &&
doesNotContain('\r') ->
doesNotContain('\r') -> {
"\r\n".also { LOGGER.trace { "line separator: ${eolEditorConfigProperty.name} --> CRLF" } }
}

else -> "\n".also { LOGGER.trace { "line separator: ${eolEditorConfigProperty.name} --> LF" } }
else -> {
"\n".also { LOGGER.trace { "line separator: ${eolEditorConfigProperty.name} --> LF" } }
}
}

private fun Code.doesNotContain(char: Char) = content.lastIndexOf(char) != -1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,23 @@ internal fun ASTNode.insertKtlintRuleSuppression(
// - otherwise to the @SuppressWarnings annotation if found
// - otherwise create a new @Suppress annotation
when {
suppressionAnnotations.containsKey(SuppressAnnotationType.SUPPRESS) ->
suppressionAnnotations.containsKey(SuppressAnnotationType.SUPPRESS) -> {
fullyQualifiedSuppressionIds.mergeInto(
suppressionAnnotations.getValue(SuppressAnnotationType.SUPPRESS),
SuppressAnnotationType.SUPPRESS,
)
}

suppressionAnnotations.containsKey(SuppressAnnotationType.SUPPRESS_WARNINGS) ->
suppressionAnnotations.containsKey(SuppressAnnotationType.SUPPRESS_WARNINGS) -> {
fullyQualifiedSuppressionIds.mergeInto(
suppressionAnnotations.getValue(SuppressAnnotationType.SUPPRESS_WARNINGS),
SuppressAnnotationType.SUPPRESS_WARNINGS,
)
}

else -> targetASTNode.createSuppressAnnotation(SuppressAnnotationType.SUPPRESS, fullyQualifiedSuppressionIds)
else -> {
targetASTNode.createSuppressAnnotation(SuppressAnnotationType.SUPPRESS, fullyQualifiedSuppressionIds)
}
}
}

Expand Down Expand Up @@ -400,9 +404,13 @@ internal fun String.isKtlintSuppressionId() = removePrefix(DOUBLE_QUOTE).startsW

internal fun String.toFullyQualifiedKtlintSuppressionId(): String =
when (this) {
KTLINT_SUPPRESSION_ID_ALL_RULES -> this
KTLINT_SUPPRESSION_ID_ALL_RULES -> {
this
}

KTLINT_PREFIX -> this.surroundWith(DOUBLE_QUOTE)
KTLINT_PREFIX -> {
this.surroundWith(DOUBLE_QUOTE)
}

else -> {
removeSurrounding(DOUBLE_QUOTE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ private class SegmentTree(
r: Int,
): Int =
when {
l > r -> -1
l > r -> {
-1
}

else -> {
val i = l + (r - l) / 2
Expand Down
Loading

0 comments on commit 50e44a7

Please sign in to comment.