Skip to content

Commit 44bf14c

Browse files
nreid260facebook-github-bot
authored andcommitted
For --google_style, break between ( and long condition expressions (#325)
Summary: Fixes #319 Pull Request resolved: #325 Reviewed By: strulovich Differential Revision: D36929348 Pulled By: cgrushko fbshipit-source-id: 48f649a7edf08e58cdea7b6a620132c62807c0e4
1 parent 24e6942 commit 44bf14c

File tree

2 files changed

+114
-38
lines changed

2 files changed

+114
-38
lines changed

core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt

+29-34
Original file line numberDiff line numberDiff line change
@@ -1754,18 +1754,8 @@ class KotlinInputAstVisitor(
17541754
override fun visitWhenExpression(expression: KtWhenExpression) {
17551755
builder.sync(expression)
17561756
builder.block(ZERO) {
1757-
builder.token("when")
1758-
expression.subjectExpression?.let { subjectExp ->
1759-
builder.space()
1760-
builder.block(ZERO) {
1761-
builder.token("(")
1762-
builder.block(if (isGoogleStyle) expressionBreakIndent else ZERO) { visit(subjectExp) }
1763-
if (isGoogleStyle) {
1764-
builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO)
1765-
}
1766-
}
1767-
builder.token(")")
1768-
}
1757+
emitKeywordWithCondition("when", expression.subjectExpression)
1758+
17691759
builder.space()
17701760
builder.token("{", Doc.Token.RealOrImaginary.REAL, blockIndent, Optional.of(blockIndent))
17711761

@@ -1837,18 +1827,7 @@ class KotlinInputAstVisitor(
18371827
override fun visitIfExpression(expression: KtIfExpression) {
18381828
builder.sync(expression)
18391829
builder.block(ZERO) {
1840-
builder.block(ZERO) {
1841-
builder.token("if")
1842-
builder.space()
1843-
builder.token("(")
1844-
builder.block(if (isGoogleStyle) expressionBreakIndent else ZERO) {
1845-
visit(expression.condition)
1846-
}
1847-
if (isGoogleStyle) {
1848-
builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO)
1849-
}
1850-
}
1851-
builder.token(")")
1830+
emitKeywordWithCondition("if", expression.condition)
18521831

18531832
if (expression.then is KtBlockExpression) {
18541833
builder.space()
@@ -2043,11 +2022,7 @@ class KotlinInputAstVisitor(
20432022
/** Example `while (a < b) { ... }` */
20442023
override fun visitWhileExpression(expression: KtWhileExpression) {
20452024
builder.sync(expression)
2046-
builder.token("while")
2047-
builder.space()
2048-
builder.token("(")
2049-
visit(expression.condition)
2050-
builder.token(")")
2025+
emitKeywordWithCondition("while", expression.condition)
20512026
builder.space()
20522027
visit(expression.body)
20532028
}
@@ -2061,11 +2036,7 @@ class KotlinInputAstVisitor(
20612036
visit(expression.body)
20622037
builder.space()
20632038
}
2064-
builder.token("while")
2065-
builder.space()
2066-
builder.token("(")
2067-
visit(expression.condition)
2068-
builder.token(")")
2039+
emitKeywordWithCondition("while", expression.condition)
20692040
}
20702041

20712042
/** Example `break` or `break@foo` in a loop */
@@ -2436,4 +2407,28 @@ class KotlinInputAstVisitor(
24362407
private fun visit(element: PsiElement?) {
24372408
element?.accept(this)
24382409
}
2410+
2411+
/** Emits a key word followed by a condition, e.g. `if (b)` or `while (c < d )` */
2412+
private fun emitKeywordWithCondition(keyword: String, condition: KtExpression?) {
2413+
if (condition == null) {
2414+
builder.token(keyword)
2415+
return
2416+
}
2417+
2418+
builder.block(ZERO) {
2419+
builder.token(keyword)
2420+
builder.space()
2421+
builder.token("(")
2422+
if (isGoogleStyle) {
2423+
builder.block(expressionBreakIndent) {
2424+
builder.breakOp(Doc.FillMode.UNIFIED, "", ZERO)
2425+
visit(condition)
2426+
builder.breakOp(Doc.FillMode.UNIFIED, "", expressionBreakNegativeIndent)
2427+
}
2428+
} else {
2429+
builder.block(ZERO) { visit(condition) }
2430+
}
2431+
}
2432+
builder.token(")")
2433+
}
24392434
}

core/src/test/java/com/facebook/ktfmt/format/GoogleStyleFormatterKtTest.kt

+85-4
Original file line numberDiff line numberDiff line change
@@ -823,14 +823,16 @@ class GoogleStyleFormatterKtTest {
823823
"""
824824
|----------------------------
825825
|fun foo() {
826-
| if (expressions1 &&
826+
| if (
827+
| expressions1 &&
827828
| expression2 &&
828829
| expression3
829830
| ) {
830831
| bar()
831832
| }
832833
|
833-
| if (foo(
834+
| if (
835+
| foo(
834836
| expressions1 &&
835837
| expression2 &&
836838
| expression3
@@ -843,21 +845,39 @@ class GoogleStyleFormatterKtTest {
843845
formattingOptions = Formatter.GOOGLE_FORMAT,
844846
deduceMaxWidth = true)
845847

848+
@Test
849+
fun `if expression with condition that exactly fits to line`() =
850+
assertFormatted(
851+
"""
852+
|-------------------------
853+
|fun foo() {
854+
| if (
855+
| e1 && e2 && e3 = e4
856+
| ) {
857+
| bar()
858+
| }
859+
|}
860+
|""".trimMargin(),
861+
formattingOptions = Formatter.GOOGLE_FORMAT,
862+
deduceMaxWidth = true)
863+
846864
@Test
847865
fun `when() expression with multiline condition`() =
848866
assertFormatted(
849867
"""
850868
|-----------------------
851869
|fun foo() {
852-
| when (expressions1 +
870+
| when (
871+
| expressions1 +
853872
| expression2 +
854873
| expression3
855874
| ) {
856875
| 1 -> print(1)
857876
| 2 -> print(2)
858877
| }
859878
|
860-
| when (foo(
879+
| when (
880+
| foo(
861881
| expressions1 &&
862882
| expression2 &&
863883
| expression3
@@ -871,6 +891,67 @@ class GoogleStyleFormatterKtTest {
871891
formattingOptions = Formatter.GOOGLE_FORMAT,
872892
deduceMaxWidth = true)
873893

894+
@Test
895+
fun `when expression with condition that exactly fits to line`() =
896+
assertFormatted(
897+
"""
898+
|---------------------------
899+
|fun foo() {
900+
| when (
901+
| e1 && e2 && e3 = e4
902+
| ) {
903+
| 1 -> print(1)
904+
| 2 -> print(2)
905+
| }
906+
|}
907+
|""".trimMargin(),
908+
formattingOptions = Formatter.GOOGLE_FORMAT,
909+
deduceMaxWidth = true)
910+
911+
@Test
912+
fun `while expression with multiline condition`() =
913+
assertFormatted(
914+
"""
915+
|----------------------------
916+
|fun foo() {
917+
| while (
918+
| expressions1 &&
919+
| expression2 &&
920+
| expression3
921+
| ) {
922+
| bar()
923+
| }
924+
|
925+
| while (
926+
| foo(
927+
| expressions1 &&
928+
| expression2 &&
929+
| expression3
930+
| )
931+
| ) {
932+
| bar()
933+
| }
934+
|}
935+
|""".trimMargin(),
936+
formattingOptions = Formatter.GOOGLE_FORMAT,
937+
deduceMaxWidth = true)
938+
939+
@Test
940+
fun `while expression with condition that exactly fits to line`() =
941+
assertFormatted(
942+
"""
943+
|----------------------------
944+
|fun foo() {
945+
| while (
946+
| e1 && e2 && e3 = e4
947+
| ) {
948+
| bar()
949+
| }
950+
|}
951+
|""".trimMargin(),
952+
formattingOptions = Formatter.GOOGLE_FORMAT,
953+
deduceMaxWidth = true)
954+
874955
@Test
875956
fun `handle destructuring declaration`() =
876957
assertFormatted(

0 commit comments

Comments
 (0)