Skip to content

Commit

Permalink
Add function expression body rule (#2151)
Browse files Browse the repository at this point in the history
This rule rewrites function bodies only containining a `return` or `throw` expression to an expression body.

Closes #2150
  • Loading branch information
paul-dingemans authored Jul 28, 2023
1 parent 305901c commit 9c9ca21
Show file tree
Hide file tree
Showing 26 changed files with 439 additions and 84 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).

### Added

* Add rule `class-signature`. This rule rewrites the class header to a consistent format. In code style `ktlint_official`, super types are always wrapped to a separate line. In other code styles, super types are only wrapped in classes having multiple super types. Especially for code style `ktlint_official` the class headers are rewritten in a more consistent format. See [examples in documentation](https://pinterest.github.io/ktlint/latest/rules/experimental/#class-signature). `class-signature` [#875](https://github.com/pinterest/ktlint/issues/1349), [#1349](https://github.com/pinterest/ktlint/issues/875)
* Add experimental rule `class-signature`. This rule rewrites the class header to a consistent format. In code style `ktlint_official`, super types are always wrapped to a separate line. In other code styles, super types are only wrapped in classes having multiple super types. Especially for code style `ktlint_official` the class headers are rewritten in a more consistent format. See [examples in documentation](https://pinterest.github.io/ktlint/latest/rules/experimental/#class-signature). `class-signature` [#875](https://github.com/pinterest/ktlint/issues/1349), [#1349](https://github.com/pinterest/ktlint/issues/875)
* Add experimental rule `function-expression-body`. This rule rewrites function bodies only contain a `return` or `throw` expression to an expression body. [#2150](https://github.com/pinterest/ktlint/issues/2150)

### Removed
* As a part of public API stabilization, data classes are no longer used in the public API. As of that, functions like `copy()` or `componentN()` (used for destructuring declarations) are not available anymore. This is a binary incompatible change, breaking backwards compatibility. ([#2133](https://github.com/pinterest/ktlint/issues/2133))
Expand Down Expand Up @@ -35,7 +36,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
* Update dependency com.google.jimfs:jimfs to v1.3.0 ([#2112](https://github.com/pinterest/ktlint/pull/2112))
* As a part of public API stabilization, configure `binary-compatibility-validator` plugin for compile-time verification of binary compatibility with previous `ktlint` versions ([#2131](https://github.com/pinterest/ktlint/pull/2131))
* Update dependency org.junit.jupiter:junit-jupiter to v5.10.0 ([#2148](https://github.com/pinterest/ktlint/pull/2148))
*

## [0.50.0] - 2023-06-29

### Deprecation of ktlint-enable and ktlint-disable directives
Expand Down
49 changes: 49 additions & 0 deletions documentation/snapshot/docs/rules/experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,55 @@ This rule is only run when `ktlint_code_style` is set to `ktlint_official` or wh

## Function signature

Rewrites a function body only containing a `return` or `throw` expression to an expression body.

!!! note:
If the function body contains a comment, it is not rewritten to an expression body.

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

```kotlin
fun foo1() = "foo"
fun foo2(): String = "foo"
fun foo3(): Unit = throw IllegalArgumentException("some message")
fun foo4(): Foo = throw IllegalArgumentException("some message")
fun foo5() {
return "foo" // some comment
}
fun foo6(): String {
/* some comment */
return "foo"
}
fun foo7() {
throw IllegalArgumentException("some message")
/* some comment */
}
fun foo8(): Foo {
throw IllegalArgumentException("some message")
// some comment
}
```
=== "[:material-heart-off-outline:](#) Disallowed"

```kotlin
fun foo1() {
return "foo"
}
fun foo2(): String {
return "foo"
}
fun foo3() {
throw IllegalArgumentException("some message")
}
fun foo4(): Foo {
throw IllegalArgumentException("some message")
}
```

Rule id: `function-expression-body` (`standard` rule set)

## Function signature

Rewrites the function signature to a single line when possible (e.g. when not exceeding the `max_line_length` property) or a multiline signature otherwise. In case of function with a body expression, the body expression is placed on the same line as the function signature when not exceeding the `max_line_length` property. Optionally the function signature can be forced to be written as a multiline signature in case the function has more than a specified number of parameters (`.editorconfig` property `ktlint_function_signature_wrapping_rule_always_with_minimum_parameters`)

=== "[:material-heart:](#) Ktlint"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ public class FormatReporterProvider : ReporterProviderV2<FormatReporter> {

private fun String.emptyOrTrue() = this == "" || this == "true"

private fun getColor(color: String?): Color {
return Color.values().firstOrNull { it.name == color } ?: throw IllegalArgumentException("Invalid color parameter.")
}
private fun getColor(color: String?): Color =
Color.values().firstOrNull {
it.name == color
} ?: throw IllegalArgumentException("Invalid color parameter.")
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ public class PlainReporterProvider : ReporterProviderV2<PlainReporter> {

private fun String.emptyOrTrue() = this == "" || this == "true"

private fun getColor(colorInput: String?): Color {
return Color.values().firstOrNull { it.name == colorInput } ?: throw IllegalArgumentException("Invalid color parameter.")
}
private fun getColor(colorInput: String?): Color =
Color.values().firstOrNull {
it.name == colorInput
} ?: throw IllegalArgumentException("Invalid color parameter.")
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,14 @@ internal fun FileSystem.fileSequence(
override fun preVisitDirectory(
dirPath: Path,
dirAttr: BasicFileAttributes,
): FileVisitResult {
return if (Files.isHidden(dirPath)) {
): FileVisitResult =
if (Files.isHidden(dirPath)) {
LOGGER.trace { "- Dir: $dirPath: Ignore" }
FileVisitResult.SKIP_SUBTREE
} else {
LOGGER.trace { "- Dir: $dirPath: Traverse" }
FileVisitResult.CONTINUE
}
}
},
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,28 +651,18 @@ internal class KtlintCommandLine {
) {
val pill =
object : Future<T> {
override fun isDone(): Boolean {
throw UnsupportedOperationException()
}
override fun isDone(): Boolean = throw UnsupportedOperationException()

override fun get(
timeout: Long,
unit: TimeUnit,
): T {
throw UnsupportedOperationException()
}
): T = throw UnsupportedOperationException()

override fun get(): T {
throw UnsupportedOperationException()
}
override fun get(): T = throw UnsupportedOperationException()

override fun cancel(mayInterruptIfRunning: Boolean): Boolean {
throw UnsupportedOperationException()
}
override fun cancel(mayInterruptIfRunning: Boolean): Boolean = throw UnsupportedOperationException()

override fun isCancelled(): Boolean {
throw UnsupportedOperationException()
}
override fun isCancelled(): Boolean = throw UnsupportedOperationException()
}
val q = ArrayBlockingQueue<Future<T>>(numberOfThreads)
val producer =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ public class RuleProvider private constructor(
/**
* Creates a new [Rule] instance.
*/
public fun createNewRuleInstance(): Rule {
return provider()
}
public fun createNewRuleInstance(): Rule = provider()

/**
* Lambda which creates a new instance of the [Rule]. Important: to ensure that a [Rule] can keep internal state and that processing of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,12 @@ public class EditorConfig(
}
}

private fun Set<EditorConfigProperty<*>>.defaultProperties(): Map<String, Property> {
return associate { editorConfigProperty ->
private fun Set<EditorConfigProperty<*>>.defaultProperties(): Map<String, Property> =
associate { editorConfigProperty ->
editorConfigProperty
.writeDefaultValue()
.let { editorConfigProperty.name to editorConfigProperty.toPropertyWithValue(it) }
}
}

private fun <T> EditorConfigProperty<T>.writeDefaultValue() = propertyWriter(getDefaultValue())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,11 @@ internal class RuleExecutionContext private constructor(
)
}

private fun normalizeText(text: String): String {
return text
private fun normalizeText(text: String): String =
text
.replace("\r\n", "\n")
.replace("\r", "\n")
.replaceFirst(UTF8_BOM, "")
}

private fun PsiElement.findErrorElement(): PsiErrorElement? {
if (this is PsiErrorElement) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ internal class RunAfterRuleFilter : RuleFilter {
private fun Set<RuleProvider>.canRunWith(loadedRuleProviders: Set<RuleProvider>): Set<RuleProvider> =
canRunWithRuleIds(loadedRuleProviders.map { it.ruleId }.toSet())

private fun Set<RuleProvider>.canRunWithRuleIds(loadedRuleIds: Set<RuleId>): Set<RuleProvider> {
return this
private fun Set<RuleProvider>.canRunWithRuleIds(loadedRuleIds: Set<RuleId>): Set<RuleProvider> =
this
.filter { it.canRunWith(loadedRuleIds) }
.let { unblockedRuleProviders ->
if (unblockedRuleProviders.isEmpty()) {
Expand All @@ -165,7 +165,6 @@ internal class RunAfterRuleFilter : RuleFilter {
.plus(unblockedRuleProviders)
}
}.toSet()
}

private fun RuleProvider.canRunWith(loadedRuleIds: Set<RuleId>): Boolean =
this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ public class KtlintSuppressionRule(
private fun ASTNode.createFileAnnotation(
suppressType: SuppressAnnotationType,
suppressions: Set<String>,
): ASTNode {
return suppressions
): ASTNode =
suppressions
.sorted()
.joinToString()
.let { sortedSuppressions -> "@file:${suppressType.annotationName}($sortedSuppressions)" }
Expand All @@ -468,7 +468,6 @@ public class KtlintSuppressionRule(
?.firstChild
?: throw IllegalStateException("Can not create annotation '$annotation'")
}.node
}

private fun ASTNode.createFileAnnotationList(annotation: ASTNode) {
require(isRoot()) { "File annotation list can only be created for root node" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,10 +468,9 @@ class RuleProviderSorterTest {
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
): Unit =
throw UnsupportedOperationException(
"Rule should never be really invoked because that is not the aim of this unit test.",
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,10 @@ class InternalRuleProvidersFilterTest {
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
): Unit =
throw UnsupportedOperationException(
"Rule should never be really invoked because that is not the aim of this unit test.",
)
}
}

private fun Set<RuleProvider>.toRuleId() = map { it.ruleId }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,11 +373,10 @@ class RunAfterRuleFilterTest {
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
): Unit =
throw UnsupportedOperationException(
"Rule should never be really invoked because that is not the aim of this unit test.",
)
}
}

private fun createRuleProviders(vararg rules: Rule) =
Expand Down
10 changes: 10 additions & 0 deletions ktlint-ruleset-standard/api/ktlint-ruleset-standard.api
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,16 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/FunKeywordSpacing
public static final fun getFUN_KEYWORD_SPACING_RULE ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}

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

public final class com/pinterest/ktlint/ruleset/standard/rules/FunctionExpressionBodyRule : 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/FunctionLiteralKt {
public static final fun getFUNCTION_LITERAL_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.pinterest.ktlint.ruleset.standard.rules.EnumWrappingRule
import com.pinterest.ktlint.ruleset.standard.rules.FilenameRule
import com.pinterest.ktlint.ruleset.standard.rules.FinalNewlineRule
import com.pinterest.ktlint.ruleset.standard.rules.FunKeywordSpacingRule
import com.pinterest.ktlint.ruleset.standard.rules.FunctionExpressionBodyRule
import com.pinterest.ktlint.ruleset.standard.rules.FunctionLiteralRule
import com.pinterest.ktlint.ruleset.standard.rules.FunctionNamingRule
import com.pinterest.ktlint.ruleset.standard.rules.FunctionReturnTypeSpacingRule
Expand Down Expand Up @@ -106,6 +107,7 @@ public class StandardRuleSetProvider : RuleSetProviderV3(RuleSetId.STANDARD) {
RuleProvider { EnumWrappingRule() },
RuleProvider { FilenameRule() },
RuleProvider { FinalNewlineRule() },
RuleProvider { FunctionExpressionBodyRule() },
RuleProvider { FunctionLiteralRule() },
RuleProvider { FunctionNamingRule() },
RuleProvider { FunctionReturnTypeSpacingRule() },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,13 @@ public class ArgumentListWrappingRule :
else -> throw UnsupportedOperationException()
}

private fun ASTNode.textContainsIgnoringLambda(char: Char): Boolean {
return children().any { child ->
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) }
}
}

private fun ASTNode.hasTypeArgumentListInFront(): Boolean =
treeParent.children()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,9 @@ public class ChainWrappingRule :

private fun ASTNode.isInPrefixPosition() = treeParent?.treeParent?.elementType == PREFIX_EXPRESSION

private fun ASTNode.isElvisOperatorAndComment(): Boolean {
return elementType == ELVIS &&
private fun ASTNode.isElvisOperatorAndComment(): Boolean =
elementType == ELVIS &&
leaves().takeWhile { it.isWhiteSpaceWithoutNewline() || it.isPartOfComment() }.any()
}
}

public val CHAIN_WRAPPING_RULE_ID: RuleId = ChainWrappingRule().ruleId
Loading

0 comments on commit 9c9ca21

Please sign in to comment.