diff --git a/.gitignore b/.gitignore index 5099988d..1b24da0a 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,4 @@ release-ktfmt-website/ .gradle **/build/ !src/**/build/ +.claude/ diff --git a/README.md b/README.md index d6381b98..c1bed148 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,32 @@ $ java -jar /path/to/ktfmt--with-dependencies.jar [--kotlinlang-style | `--kotlinlang-style` makes `ktfmt` use a block indent of 4 spaces instead of 2. See below for details. +#### Cascading Lambda Breaks + +For DSLs like Jetpack Compose where nested lambdas represent semantic hierarchies, you can use the `--cascade-nested-lambda-breaks` option to preserve visual structure: + +``` +$ java -jar /path/to/ktfmt--with-dependencies.jar --cascade-nested-lambda-breaks [files...] +``` + +When enabled, nested trailing lambdas are forced to multi-line format when their parent breaks, ensuring complete visual hierarchy: + +```kotlin +// With --cascade-nested-lambda-breaks +App { + SelectableCard { + Button { + Text("Click me") + } + } +} + +// Without (default) +App { SelectableCard { Button { Text("Click me") } } } +``` + +This option only affects trailing lambdas and does not change the formatting of lambdas in other contexts (like `map` or `filter` chains). + ***Note:*** *There is no configurability as to the formatter's algorithm for formatting (apart from the different styles). This is a deliberate design decision to unify our code formatting on a single diff --git a/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt b/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt index 8eff0047..395347a6 100644 --- a/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt +++ b/core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt @@ -72,15 +72,17 @@ data class ParsedArgs( |Commands options: | -h, --help Show this help message | -v, --version Show version - | -n, --dry-run Don't write to files, only report files which + | -n, --dry-run Don't write to files, only report files which | would have changed | --meta-style Use 2-space block indenting (default) | --google-style Google internal style (2 spaces) | --kotlinlang-style Kotlin language guidelines style (4 spaces) | --stdin-name= Name to report when formatting code from stdin - | --set-exit-if-changed Sets exit code to 1 if any input file was not + | --set-exit-if-changed Sets exit code to 1 if any input file was not | formatted/touched | --do-not-remove-unused-imports Leaves all imports in place, even if not used + | --cascade-nested-lambda-breaks Force nested trailing lambdas to break when parent + | breaks (useful for Compose DSLs) | |ARGFILE: | If the only argument begins with '@', the remainder of the argument is treated @@ -120,6 +122,9 @@ data class ParsedArgs( arg == "--dry-run" || arg == "-n" -> dryRun = true arg == "--set-exit-if-changed" -> setExitIfChanged = true arg == "--do-not-remove-unused-imports" -> removeUnusedImports = false + arg == "--cascade-nested-lambda-breaks" -> { + formattingOptions = formattingOptions.copy(cascadeNestedLambdaBreaks = true) + } arg.startsWith("--stdin-name=") -> stdinName = parseKeyValueArg("--stdin-name", arg) diff --git a/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt b/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt index caa6a584..c38c53fc 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/FormattingOptions.kt @@ -57,6 +57,31 @@ data class FormattingOptions( /** Whether ktfmt should remove imports that are not used. */ val removeUnusedImports: Boolean = true, + /** + * Whether to cascade (propagate) lambda break decisions to nested trailing lambdas. + * + * When enabled, if a parent lambda breaks to multiple lines, all nested trailing lambdas will + * also be forced to break, preserving the visual hierarchy. This is particularly useful for + * DSLs like Jetpack Compose where nesting represents semantic parent-child relationships. + * + * For example, when enabled: + * ``` + * App { + * SelectableCard { + * Button { + * Text("") // Forced to multi-line despite being simple + * } + * } + * } + * ``` + * + * When disabled (default), the same code might format as: + * ``` + * App { SelectableCard { Button { Text("") }}} + * ``` + */ + val cascadeNestedLambdaBreaks: Boolean = false, + /** * Print the Ops generated by KotlinInputAstVisitor to help reason about formatting (i.e., * newline) decisions diff --git a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt index 1cc8b11f..bb4ec5a6 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt @@ -138,6 +138,12 @@ class KotlinInputAstVisitor( private val builder: OpsBuilder, ) : KtTreeVisitorVoid() { + /** + * Track whether we're currently inside a lambda that has broken to multi-line. Used when + * cascadeNestedLambdaBreaks is enabled to force nested trailing lambdas to break. + */ + private var insideBreakingLambda: Boolean = false + /** Standard indentation for a block */ private val blockIndent: Indent.Const = Indent.Const.make(options.blockIndent, 1) @@ -863,10 +869,15 @@ class KotlinInputAstVisitor( * car() * } * ``` + * + * @param parentLambdaBroke used when cascadeNestedLambdaBreaks is enabled to track whether the + * parent lambda has broken to multi-line, so that nested trailing lambdas can be forced to + * break as well */ private fun visitLambdaExpressionInternal( lambdaExpression: KtLambdaExpression, brokeBeforeBrace: BreakTag?, + parentLambdaBroke: Boolean = false, ) { builder.sync(lambdaExpression) @@ -923,15 +934,50 @@ class KotlinInputAstVisitor( builder.breakOp(Doc.FillMode.UNIFIED, "", bracePlusBlockIndent) builder.block(bracePlusBlockIndent) { builder.blankLineWanted(OpsBuilder.BlankLineWanted.NO) + + // FORCE multi-line when parent lambda broke and option is enabled + val shouldForceMultiline = + options.cascadeNestedLambdaBreaks && + parentLambdaBroke && + isTrailingLambda(lambdaExpression) + + // Determine if this lambda will break to multi-line + // A lambda breaks when it either: + // 1. Is forced to break by the cascadeNestedLambdaBreaks option (parent broke) + // 2. Contains nested trailing lambdas and cascadeNestedLambdaBreaks is enabled + // 3. Has multiple statements + // 4. Has a return expression + // 5. Has a comment before the first statement + val currentLambdaWillBreak = + shouldForceMultiline || + (options.cascadeNestedLambdaBreaks && + isTrailingLambda(lambdaExpression) && + containsTrailingLambdas(lambdaExpression)) || + expressionStatements.size > 1 || + expressionStatements.firstOrNull() is KtReturnExpression || + bodyExpression.startsWithComment() + + // Track lambda break state for nested lambdas + val previousInsideBreakingLambda = insideBreakingLambda + if (options.cascadeNestedLambdaBreaks && isTrailingLambda(lambdaExpression)) { + insideBreakingLambda = currentLambdaWillBreak + } + if ( - expressionStatements.size == 1 && + !shouldForceMultiline && + expressionStatements.size == 1 && expressionStatements.first() !is KtReturnExpression && !bodyExpression.startsWithComment() ) { visitStatement(expressionStatements[0]) } else { + // FORCED multi-line: either naturally multi-line OR forced by parent visitStatements(expressionStatements) } + + // Restore previous state + insideBreakingLambda = previousInsideBreakingLambda + builder.breakOp(Doc.FillMode.UNIFIED, " ", bracePlusZeroIndent) } } @@ -1162,6 +1208,7 @@ class KotlinInputAstVisitor( visitLambdaExpressionInternal( argument.getArgumentExpression() as KtLambdaExpression, brokeBeforeBrace = brokeBeforeBrace, + parentLambdaBroke = insideBreakingLambda, ) } else { visit(argument.getArgumentExpression()) @@ -1501,6 +1548,55 @@ class KotlinInputAstVisitor( return false } + /** + * Determines if the given expression is a trailing lambda in a call expression. + * + * This is used to identify lambdas that should inherit hierarchy preservation when + * cascadeNestedLambdaBreaks is enabled. + */ + private fun isTrailingLambda(expression: KtExpression?): Boolean { + if (expression == null) return false + val parent = expression.parent + + // Check if this lambda is in the lambdaArguments list of a call expression + if (parent is KtLambdaArgument) { + val callExpression = parent.parent as? KtCallExpression + return callExpression?.lambdaArguments?.contains(parent) == true + } + + // Check if this is a labeled lambda that's a trailing lambda + if (expression is KtLabeledExpression) { + return isTrailingLambda(expression.parent as? KtExpression) + } + + return false + } + + /** + * Determines if a lambda expression contains trailing lambda calls in its body. + * + * This is used to detect when a lambda should be forced to break because it contains nested + * trailing lambdas that form a hierarchy needing preservation. + */ + private fun containsTrailingLambdas(lambdaExpression: KtLambdaExpression): Boolean { + val body = lambdaExpression.bodyExpression ?: return false + + // Check if any element in the body is a call expression with trailing lambdas + fun checkElement(element: PsiElement): Boolean { + if (element is KtCallExpression && element.lambdaArguments.isNotEmpty()) { + return true + } + for (child in element.children) { + if (checkElement(child)) { + return true + } + } + return false + } + + return checkElement(body) + } + /** See [isLambdaOrScopingFunction] for examples. */ private fun visitLambdaOrScopingFunction(expr: PsiElement?) { val breakToExpr = genSym() @@ -1517,7 +1613,11 @@ class KotlinInputAstVisitor( carry = carry.baseExpression ?: fail() } if (carry is KtLambdaExpression) { - visitLambdaExpressionInternal(carry, brokeBeforeBrace = breakToExpr) + visitLambdaExpressionInternal( + carry, + brokeBeforeBrace = breakToExpr, + parentLambdaBroke = insideBreakingLambda, + ) return } diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt index 264e32ec..4a356209 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt @@ -8610,6 +8610,251 @@ class FormatterTest { assertThatFormatting(code).isEqualTo(expected) } + @Test + fun `cascade nested lambda breaks - force nested lambda hierarchy when option enabled`() { + val code = + """ + |fun compose() { + | App { SelectableCard { Button { Text("Hello") } } } + |} + |""" + .trimMargin() + + val expected = + """ + |fun compose() { + | App { + | SelectableCard { + | Button { + | Text("Hello") + | } + | } + | } + |} + |""" + .trimMargin() + + assertThatFormatting(code) + .withOptions( + FormattingOptions( + cascadeNestedLambdaBreaks = true, + blockIndent = 2, + continuationIndent = 4, + ) + ) + .isEqualTo(expected) + } + + @Test + fun `cascade nested lambda breaks - maintain single line when option disabled`() { + val code = + """ + |fun compose() { + | App { SelectableCard { Button { Text("Hello") } } } + |} + |""" + .trimMargin() + + val expected = + """ + |fun compose() { + | App { SelectableCard { Button { Text("Hello") } } } + |} + |""" + .trimMargin() + + assertThatFormatting(code) + .withOptions( + FormattingOptions( + cascadeNestedLambdaBreaks = false, + blockIndent = 2, + continuationIndent = 4, + ) + ) + .isEqualTo(expected) + } + + @Test + fun `cascade nested lambda breaks - force hierarchy only for trailing lambdas`() { + val code = + """ + |fun test() { + | val result = listOf(1, 2, 3).map { it * 2 }.filter { it > 2 } + | Container { + | Box { + | Text("Hi") + | } + | } + |} + |""" + .trimMargin() + + val expected = + """ + |fun test() { + | val result = listOf(1, 2, 3).map { it * 2 }.filter { it > 2 } + | Container { + | Box { + | Text("Hi") + | } + | } + |} + |""" + .trimMargin() + + assertThatFormatting(code) + .withOptions( + FormattingOptions( + cascadeNestedLambdaBreaks = true, + blockIndent = 2, + continuationIndent = 4, + ) + ) + .isEqualTo(expected) + } + + @Test + fun `cascade nested lambda breaks - demonstrate forcing behavior with parent break`() { + val code = + """ + |fun test() { + | Container { + | val x = 1 + | Card { Text("x") } + | } + | Container { Card { Text("x") } } + |} + |""" + .trimMargin() + + val expected = + """ + |fun test() { + | Container { + | val x = 1 + | Card { + | Text("x") + | } + | } + | Container { + | Card { + | Text("x") + | } + | } + |} + |""" + .trimMargin() + + assertThatFormatting(code) + .withOptions( + FormattingOptions( + cascadeNestedLambdaBreaks = true, + blockIndent = 2, + continuationIndent = 4, + ) + ) + .isEqualTo(expected) + } + + @Test + fun `cascade nested lambda breaks - preserve hierarchy with multiple statements`() { + val code = + """ + |fun compose() { + | App { + | val state = remember { mutableStateOf(0) } + | SelectableCard { + | Button { + | state.value++ + | Text("Count: ${'$'}{state.value}") + | } + | } + | } + |} + |""" + .trimMargin() + + val expected = + """ + |fun compose() { + | App { + | val state = remember { + | mutableStateOf(0) + | } + | SelectableCard { + | Button { + | state.value++ + | Text("Count: ${'$'}{state.value}") + | } + | } + | } + |} + |""" + .trimMargin() + + assertThatFormatting(code) + .withOptions( + FormattingOptions( + cascadeNestedLambdaBreaks = true, + blockIndent = 2, + continuationIndent = 4, + ) + ) + .isEqualTo(expected) + } + + @Test + fun `cascade nested lambda breaks - handle deep nesting levels`() { + val code = + """ + |fun deepCompose() { + | Level1 { + | Level2 { + | Level3 { + | Level4 { + | Level5 { + | Text("Deep") + | } + | } + | } + | } + | } + |} + |""" + .trimMargin() + + assertThatFormatting(code) + .withOptions( + FormattingOptions( + cascadeNestedLambdaBreaks = true, + blockIndent = 2, + continuationIndent = 4, + ) + ) + .isEqualTo(code) + } + + @Test + fun `cascade nested lambda breaks - no effect on non-nested lambdas`() { + val code = + """ + |fun test() { + | singleCall { doSomething() } + |} + |""" + .trimMargin() + + assertThatFormatting(code) + .withOptions( + FormattingOptions( + cascadeNestedLambdaBreaks = true, + blockIndent = 2, + continuationIndent = 4, + ) + ) + .isEqualTo(code) + } + companion object { /** Triple quotes, useful to use within triple-quoted strings. */ private const val TQ = "\"\"\""