Skip to content

Commit 5c46e27

Browse files
Nivaldo Bondançafacebook-github-bot
authored andcommitted
Do not special format multiline strings with template expressions
Summary: This should help fix the issue raised by #556 Reviewed By: cortinico Differential Revision: D81201747 fbshipit-source-id: f75c7c449c67eadf2118af94e4939cbb7ced84fc
1 parent bb6ad1e commit 5c46e27

3 files changed

Lines changed: 75 additions & 29 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,18 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
1313
## [Unreleased]
1414

1515
### Changed
16+
- Updated ShadowJar to 9.0.2 (https://github.com/facebook/ktfmt/pull/555)
17+
18+
### Fixed
19+
- Do not apply special format handling of multiline strings with template expressions in them (https://github.com/facebook/ktfmt/issues/556)
1620

1721

1822
## [0.57]
1923

2024
### Added
2125
- `TrailingCommaManagementStrategy.ONLY_ADD` strategy that does not remove existing trailing commas (https://github.com/facebook/ktfmt/issues/461, https://github.com/facebook/ktfmt/issues/512, https://github.com/facebook/ktfmt/issues/514)
2226
- Formatting of where clauses (https://github.com/facebook/ktfmt/issues/541)
23-
- Special format handling of multiline strings with `trimMargin()` and `trimIndent` (https://github.com/facebook/ktfmt/issues/389)
27+
- Special format handling of multiline strings with `trimMargin()` and `trimIndent()` (https://github.com/facebook/ktfmt/issues/389)
2428

2529
### Changed
2630
- `FormattingOptions.manageTrailingCommas` was replaced with `FormattingOptions.trailingCommaManagementStrategy`, which also added new `TrailingCommaManagementStrategy.ONLY_ADD` strategy (https://github.com/facebook/ktfmt/issues/461, https://github.com/facebook/ktfmt/issues/512, https://github.com/facebook/ktfmt/issues/514)

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset
2727
*/
2828
class MultilineStringFormatter(val continuationIndentSize: Int) {
2929
class Candidate(
30+
val isDollarString: Boolean,
3031
val isMargin: Boolean,
3132
/* The start offset of the trim method call, right before `.trimX` */
3233
val trimMethodCallOffset: Int,
@@ -47,8 +48,8 @@ class MultilineStringFormatter(val continuationIndentSize: Int) {
4748
val candidates = getCandidates(code)
4849
val result = StringBuilder(code)
4950

50-
val openTemplateExpressionRegex =
51-
Regex("""\$\$?\{(((?<!\\)"([^"]|(\\"))*?[^\\]")|([^\\]'\\?.')|[^"'}])*$""")
51+
val simpleTemplateExpressionRegex = Regex("""\${'$'}{1}((\{?[A-Za-z_\s])|\{$)""")
52+
val dollarTemplateExpressionRegex = Regex("""\${'$'}{2}((\{?[A-Za-z_\s])|\{$)""")
5253

5354
for (candidate in candidates.sortedByDescending(Candidate::stringOffset)) {
5455
val (indentCount, lines) =
@@ -59,12 +60,12 @@ class MultilineStringFormatter(val continuationIndentSize: Int) {
5960
// Single line multiline strings are left alone
6061
continue
6162
}
62-
if (candidate.isMargin && lines.any { openTemplateExpressionRegex.find(it) != null }) {
63-
// Do not mess with multiline template expressions, as those can be a mess
64-
// Why?
65-
// 1. They span multiple lines
66-
// 2. They can be nested recursively
67-
// 3. We need to be careful as the closing character ('}') could be inside a string/char
63+
val regex =
64+
if (candidate.isDollarString) dollarTemplateExpressionRegex
65+
else simpleTemplateExpressionRegex
66+
if (lines.any { regex.find(it) != null }) {
67+
// If there are any template expressions, we cannot format the string as it's possible that
68+
// the output of the template affects the result of the trimIndent/trimMargin call
6869
continue
6970
}
7071
val indentation = " ".repeat(indentCount)
@@ -136,14 +137,15 @@ class MultilineStringFormatter(val continuationIndentSize: Int) {
136137
override fun visitQualifiedExpression(expression: KtQualifiedExpression) {
137138
val receiver = expression.receiverExpression
138139
if (receiver !is KtStringTemplateExpression) return
140+
val isDollarString = receiver.text.startsWith("$$")
139141
val selectorExpression = expression.selectorExpression?.text.orEmpty().trim()
140142
val isTrimMargin = selectorExpression.startsWith("trimMargin(")
141143
val isTrimIndent = selectorExpression.startsWith("trimIndent(")
142144
if (isTrimIndent || isTrimMargin) {
143145
// -1 here to account for the space after the dot
144146
val trimOffset = checkNotNull(expression.selectorExpression).startOffset - 1
145147
val stringOffset = receiver.startOffset
146-
candidates.add(Candidate(isTrimMargin, trimOffset, stringOffset))
148+
candidates.add(Candidate(isDollarString, isTrimMargin, trimOffset, stringOffset))
147149
}
148150
}
149151
}

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

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3259,7 +3259,7 @@ class FormatterTest {
32593259
)
32603260

32613261
@Test
3262-
fun `handles multiline trimMargin with multiline template expressions inside of it`() {
3262+
fun `handles multiline trim with template expressions inside of it`() {
32633263
assertFormatted(
32643264
"""
32653265
|val margin =
@@ -3299,7 +3299,7 @@ class FormatterTest {
32993299
| $TQ
33003300
| echo hello | wc -c
33013301
| cat hay_stack.txt | grep needle
3302-
| ${'$'}{myList.joinToString("|")}
3302+
| {myList.joinToString("|")}
33033303
| $TQ
33043304
| .trimMargin()
33053305
|"""
@@ -3311,36 +3311,76 @@ class FormatterTest {
33113311
| $TQ
33123312
| | echo hello | wc -c
33133313
| | cat hay_stack.txt | grep needle
3314-
| | ${'$'}{myList.joinToString("|")}$TQ
3314+
| | {myList.joinToString("|")}$TQ
33153315
| .trimMargin()
33163316
|"""
33173317
.trimMargin()
33183318
)
33193319
}
33203320

33213321
@Test
3322-
fun `handles multiline trimMargin with single-line template expressions`() {
3322+
fun `handles multiline trim formatting with template expressions`() {
3323+
assertFormatted(
3324+
"""
3325+
|val margin1 =
3326+
| ${TQ}my math = ${'$'}{ "}" + (1 + 2).toString() }
3327+
| | checks
3328+
| | out
3329+
| |$TQ
3330+
| .trimMargin()
3331+
|
3332+
|val margin2 =
3333+
| ${"$$"}${TQ}my math = ${"$$"}{ "}" + (1 + 2).toString() }
3334+
| | checks
3335+
| | out
3336+
| |$TQ
3337+
| .trimMargin()
3338+
|"""
3339+
.trimMargin()
3340+
)
3341+
33233342
assertThatFormatting(
33243343
"""
3325-
|val margin =
3326-
| ${TQ}my math = ${'$'}{ "}" + (1 + 2).toString() }
3327-
| | checks
3328-
| | out
3329-
| |$TQ
3330-
| .trimMargin()
3331-
|"""
3344+
|val margin1 =
3345+
| $TQ
3346+
|not_a_var$
3347+
|$1
3348+
|$\{
3349+
|$}
3350+
|$TQ
3351+
| .trimIndent()
3352+
|
3353+
|val margin2 =
3354+
| ${"$$"}$TQ
3355+
|not_a_var$$
3356+
|$$1
3357+
|$$\{
3358+
|$$}
3359+
| |$TQ
3360+
| .trimMargin()
3361+
|"""
33323362
.trimMargin()
33333363
)
33343364
.isEqualTo(
33353365
"""
3336-
|val margin =
3337-
| $TQ
3338-
| |my math = ${'$'}{ "}" + (1 + 2).toString() }
3339-
| | checks
3340-
| | out
3341-
| |$TQ
3342-
| .trimMargin()
3343-
|"""
3366+
|val margin1 =
3367+
| $TQ
3368+
| not_a_var$
3369+
| $1
3370+
| $\{
3371+
| $}
3372+
| $TQ
3373+
| .trimIndent()
3374+
|
3375+
|val margin2 =
3376+
| ${"$$"}$TQ
3377+
| |not_a_var$$
3378+
| |$$1
3379+
| |$$\{
3380+
| |$$}
3381+
| |$TQ
3382+
| .trimMargin()
3383+
|"""
33443384
.trimMargin()
33453385
)
33463386
}

0 commit comments

Comments
 (0)