From 44bf14c57a567d629a45b2c0eca9c54ce41fe8f9 Mon Sep 17 00:00:00 2001 From: nickreid Date: Tue, 7 Jun 2022 14:36:57 -0700 Subject: [PATCH] For --google_style, break between ( and long condition expressions (#325) Summary: Fixes https://github.com/facebookincubator/ktfmt/issues/319 Pull Request resolved: https://github.com/facebookincubator/ktfmt/pull/325 Reviewed By: strulovich Differential Revision: D36929348 Pulled By: cgrushko fbshipit-source-id: 48f649a7edf08e58cdea7b6a620132c62807c0e4 --- .../ktfmt/format/KotlinInputAstVisitor.kt | 63 ++++++------- .../format/GoogleStyleFormatterKtTest.kt | 89 ++++++++++++++++++- 2 files changed, 114 insertions(+), 38 deletions(-) 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 4a6aae44..490184e1 100644 --- a/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt +++ b/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt @@ -1754,18 +1754,8 @@ class KotlinInputAstVisitor( override fun visitWhenExpression(expression: KtWhenExpression) { builder.sync(expression) builder.block(ZERO) { - builder.token("when") - expression.subjectExpression?.let { subjectExp -> - builder.space() - builder.block(ZERO) { - builder.token("(") - builder.block(if (isGoogleStyle) expressionBreakIndent else ZERO) { visit(subjectExp) } - if (isGoogleStyle) { - builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO) - } - } - builder.token(")") - } + emitKeywordWithCondition("when", expression.subjectExpression) + builder.space() builder.token("{", Doc.Token.RealOrImaginary.REAL, blockIndent, Optional.of(blockIndent)) @@ -1837,18 +1827,7 @@ class KotlinInputAstVisitor( override fun visitIfExpression(expression: KtIfExpression) { builder.sync(expression) builder.block(ZERO) { - builder.block(ZERO) { - builder.token("if") - builder.space() - builder.token("(") - builder.block(if (isGoogleStyle) expressionBreakIndent else ZERO) { - visit(expression.condition) - } - if (isGoogleStyle) { - builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO) - } - } - builder.token(")") + emitKeywordWithCondition("if", expression.condition) if (expression.then is KtBlockExpression) { builder.space() @@ -2043,11 +2022,7 @@ class KotlinInputAstVisitor( /** Example `while (a < b) { ... }` */ override fun visitWhileExpression(expression: KtWhileExpression) { builder.sync(expression) - builder.token("while") - builder.space() - builder.token("(") - visit(expression.condition) - builder.token(")") + emitKeywordWithCondition("while", expression.condition) builder.space() visit(expression.body) } @@ -2061,11 +2036,7 @@ class KotlinInputAstVisitor( visit(expression.body) builder.space() } - builder.token("while") - builder.space() - builder.token("(") - visit(expression.condition) - builder.token(")") + emitKeywordWithCondition("while", expression.condition) } /** Example `break` or `break@foo` in a loop */ @@ -2436,4 +2407,28 @@ class KotlinInputAstVisitor( private fun visit(element: PsiElement?) { element?.accept(this) } + + /** Emits a key word followed by a condition, e.g. `if (b)` or `while (c < d )` */ + private fun emitKeywordWithCondition(keyword: String, condition: KtExpression?) { + if (condition == null) { + builder.token(keyword) + return + } + + builder.block(ZERO) { + builder.token(keyword) + builder.space() + builder.token("(") + if (isGoogleStyle) { + builder.block(expressionBreakIndent) { + builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO) + visit(condition) + builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakNegativeIndent) + } + } else { + builder.block(ZERO) { visit(condition) } + } + } + builder.token(")") + } } diff --git a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt index a50e7826..61b6b47b 100644 --- a/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt +++ b/core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt @@ -823,14 +823,16 @@ class GoogleStyleFormatterKtTest { """ |---------------------------- |fun foo() { - | if (expressions1 && + | if ( + | expressions1 && | expression2 && | expression3 | ) { | bar() | } | - | if (foo( + | if ( + | foo( | expressions1 && | expression2 && | expression3 @@ -843,13 +845,30 @@ class GoogleStyleFormatterKtTest { formattingOptions = Formatter.GOOGLE_FORMAT, deduceMaxWidth = true) + @Test + fun `if expression with condition that exactly fits to line`() = + assertFormatted( + """ + |------------------------- + |fun foo() { + | if ( + | e1 && e2 && e3 = e4 + | ) { + | bar() + | } + |} + |""".trimMargin(), + formattingOptions = Formatter.GOOGLE_FORMAT, + deduceMaxWidth = true) + @Test fun `when() expression with multiline condition`() = assertFormatted( """ |----------------------- |fun foo() { - | when (expressions1 + + | when ( + | expressions1 + | expression2 + | expression3 | ) { @@ -857,7 +876,8 @@ class GoogleStyleFormatterKtTest { | 2 -> print(2) | } | - | when (foo( + | when ( + | foo( | expressions1 && | expression2 && | expression3 @@ -871,6 +891,67 @@ class GoogleStyleFormatterKtTest { formattingOptions = Formatter.GOOGLE_FORMAT, deduceMaxWidth = true) + @Test + fun `when expression with condition that exactly fits to line`() = + assertFormatted( + """ + |--------------------------- + |fun foo() { + | when ( + | e1 && e2 && e3 = e4 + | ) { + | 1 -> print(1) + | 2 -> print(2) + | } + |} + |""".trimMargin(), + formattingOptions = Formatter.GOOGLE_FORMAT, + deduceMaxWidth = true) + + @Test + fun `while expression with multiline condition`() = + assertFormatted( + """ + |---------------------------- + |fun foo() { + | while ( + | expressions1 && + | expression2 && + | expression3 + | ) { + | bar() + | } + | + | while ( + | foo( + | expressions1 && + | expression2 && + | expression3 + | ) + | ) { + | bar() + | } + |} + |""".trimMargin(), + formattingOptions = Formatter.GOOGLE_FORMAT, + deduceMaxWidth = true) + + @Test + fun `while expression with condition that exactly fits to line`() = + assertFormatted( + """ + |---------------------------- + |fun foo() { + | while ( + | e1 && e2 && e3 = e4 + | ) { + | bar() + | } + |} + |""".trimMargin(), + formattingOptions = Formatter.GOOGLE_FORMAT, + deduceMaxWidth = true) + @Test fun `handle destructuring declaration`() = assertFormatted(