Do not indent raw string literals that are not followed by either tri…

…mIndent() or trimMargin() Closes pinterest#1375
paul-dingemans · Feb 25, 2022 · 1cc6e34 · 1cc6e34
1 parent a153d7e
commit 1cc6e34
Show file tree

Hide file tree

Showing 6 changed files with 166 additions and 95 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 ### Added
 
 ### Fixed
+- Do not indent raw string literals that are not followed by either trimIndent() or trimMargin() (`indent`) ([#1375](https://github.com/pinterest/ktlint/issues/1375))
 
 ### Changed
 

diff --git a/...ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRule.kt b/...ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRule.kt
@@ -7,6 +7,7 @@ import com.pinterest.ktlint.core.IndentConfig
 import com.pinterest.ktlint.core.IndentConfig.IndentStyle.SPACE
 import com.pinterest.ktlint.core.IndentConfig.IndentStyle.TAB
 import com.pinterest.ktlint.core.Rule
+import com.pinterest.ktlint.core.ast.ElementType
 import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION
 import com.pinterest.ktlint.core.ast.ElementType.ARROW
 import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION
@@ -356,14 +357,18 @@ class IndentationRule : Rule(
     }
 
     private fun rearrangeClosingQuote(
-        n: ASTNode,
+        node: ASTNode,
         autoCorrect: Boolean,
         emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
     ) {
-        val treeParent = n.treeParent
-        if (treeParent.elementType == STRING_TEMPLATE) {
-            val treeParentPsi = treeParent.psi as KtStringTemplateExpression
-            if (treeParentPsi.isMultiLine() && n.treePrev.text.isNotBlank()) {
+        node
+            .treeParent
+            .takeIf { it.elementType == STRING_TEMPLATE }
+            ?.let { it.psi as KtStringTemplateExpression }
+            ?.takeIf { it.isMultiLine() }
+            ?.takeIf { it.isFollowedByTrimIndent() || it.isFollowedByTrimMargin() }
+            ?.takeIf { node.treePrev.text.isNotBlank() }
+            ?.let {
                 // rewriting
                 // """
                 //     text
@@ -374,17 +379,16 @@ class IndentationRule : Rule(
                 // _
                 // """.trimIndent()
                 emit(
-                    n.startOffset,
+                    node.startOffset,
                     "Missing newline before \"\"\"",
                     true
                 )
                 if (autoCorrect) {
-                    n as LeafPsiElement
-                    n.rawInsertBeforeMe(LeafPsiElement(REGULAR_STRING_PART, "\n"))
+                    node as LeafPsiElement
+                    node.rawInsertBeforeMe(LeafPsiElement(REGULAR_STRING_PART, "\n"))
                 }
                 logger.trace { "$line: " + (if (!autoCorrect) "would have " else "") + "inserted newline before (closing) \"\"\"" }
             }
-        }
     }
 
     private fun mustBeFollowedByNewline(node: ASTNode): Boolean {
@@ -932,98 +936,101 @@ class IndentationRule : Rule(
         emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
         editorConfig: EditorConfig
     ) {
-        val psi = node.psi as KtStringTemplateExpression
-        if (psi.isMultiLine()) {
-            if (node.containsMixedIndentationCharacters()) {
-                // It can not be determined with certainty how mixed indentation characters should be interpreted.
-                // The trimIndent function handles tabs and spaces equally (one tabs equals one space) while the user
-                // might expect that the tab size in the indentation is more than one space.
-                emit(
-                    node.startOffset,
-                    "Indentation of multiline string should not contain both tab(s) and space(s)",
-                    false
-                )
-                return
-            }
+        node
+            .let { it.psi as KtStringTemplateExpression }
+            .takeIf { it.isFollowedByTrimIndent() || it.isFollowedByTrimMargin() }
+            ?.takeIf { it.isMultiLine() }
+            ?.let {
+                if (node.containsMixedIndentationCharacters()) {
+                    // It can not be determined with certainty how mixed indentation characters should be interpreted.
+                    // The trimIndent function handles tabs and spaces equally (one tabs equals one space) while the user
+                    // might expect that the tab size in the indentation is more than one space.
+                    emit(
+                        node.startOffset,
+                        "Indentation of multiline string should not contain both tab(s) and space(s)",
+                        false
+                    )
+                    return
+                }
 
-            val prefixLength = node.children()
-                .filterNot { it.elementType == OPEN_QUOTE }
-                .filterNot { it.elementType == CLOSING_QUOTE }
-                .filter { it.prevLeaf()?.text == "\n" }
-                .filterNot { it.text == "\n" }
-                .let { indents ->
-                    val indentsExceptBlankIndentBeforeClosingQuote = indents
-                        .filterNot { it.isIndentBeforeClosingQuote() }
-                    if (indentsExceptBlankIndentBeforeClosingQuote.count() > 0) {
-                        indentsExceptBlankIndentBeforeClosingQuote
-                    } else {
-                        indents
+                val prefixLength = node.children()
+                    .filterNot { it.elementType == OPEN_QUOTE }
+                    .filterNot { it.elementType == CLOSING_QUOTE }
+                    .filter { it.prevLeaf()?.text == "\n" }
+                    .filterNot { it.text == "\n" }
+                    .let { indents ->
+                        val indentsExceptBlankIndentBeforeClosingQuote = indents
+                            .filterNot { it.isIndentBeforeClosingQuote() }
+                        if (indentsExceptBlankIndentBeforeClosingQuote.count() > 0) {
+                            indentsExceptBlankIndentBeforeClosingQuote
+                        } else {
+                            indents
+                        }
                     }
-                }
-                .map { it.text.indentLength() }
-                .minOrNull() ?: 0
+                    .map { it.text.indentLength() }
+                    .minOrNull() ?: 0
 
-            val correctedExpectedIndent = if (node.prevLeaf()?.text == "\n") {
-                // In case the opening quotes are placed at the start of the line, then expect all lines inside the
-                // string literal and the closing quotes to have no indent as well.
-                0
-            } else {
-                expectedIndent
-            }
-            val expectedIndentation = indentConfig.indent.repeat(correctedExpectedIndent)
-            val expectedPrefixLength = correctedExpectedIndent * indentConfig.indent.length
-            node.children()
-                .forEach {
-                    if (it.prevLeaf()?.text == "\n" &&
-                        (
-                            it.isLiteralStringTemplateEntry() ||
-                                it.isVariableStringTemplateEntry() ||
-                                it.isClosingQuote()
-                            )
-                    ) {
-                        val (actualIndent, actualContent) =
-                            if (it.isIndentBeforeClosingQuote()) {
-                                it.text.splitIndentAt(it.text.length)
-                            } else if (it.isVariableStringTemplateEntry() && it.isFirstNonBlankElementOnLine()) {
-                                it.getFirstElementOnSameLine().text.splitIndentAt(expectedPrefixLength)
-                            } else {
-                                it.text.splitIndentAt(prefixLength)
-                            }
-                        if (indentConfig.containsUnexpectedIndentChar(actualIndent)) {
-                            val offsetFirstWrongIndentChar = indentConfig.indexOfFirstUnexpectedIndentChar(actualIndent)
-                            emit(
-                                it.startOffset + offsetFirstWrongIndentChar,
-                                "Unexpected '${indentConfig.unexpectedIndentCharDescription}' character(s) in margin of multiline string",
-                                true
-                            )
-                            if (autoCorrect) {
-                                (it.firstChildNode as LeafPsiElement).rawReplaceWithText(
-                                    expectedIndentation + actualContent
+                val correctedExpectedIndent = if (node.prevLeaf()?.text == "\n") {
+                    // In case the opening quotes are placed at the start of the line, then expect all lines inside the
+                    // string literal and the closing quotes to have no indent as well.
+                    0
+                } else {
+                    expectedIndent
+                }
+                val expectedIndentation = indentConfig.indent.repeat(correctedExpectedIndent)
+                val expectedPrefixLength = correctedExpectedIndent * indentConfig.indent.length
+                node.children()
+                    .forEach {
+                        if (it.prevLeaf()?.text == "\n" &&
+                            (
+                                it.isLiteralStringTemplateEntry() ||
+                                    it.isVariableStringTemplateEntry() ||
+                                    it.isClosingQuote()
                                 )
-                            }
-                        } else if (actualIndent != expectedIndentation && it.isIndentBeforeClosingQuote()) {
-                            // It is a deliberate choice not to fix the indents inside the string literal except the line which only contains
-                            // the closing quotes.
-                            emit(
-                                it.startOffset,
-                                "Unexpected indent of multiline string closing quotes",
-                                true
-                            )
-                            if (autoCorrect) {
-                                if (it.firstChildNode == null) {
-                                    (it as LeafPsiElement).rawInsertBeforeMe(
-                                        LeafPsiElement(REGULAR_STRING_PART, expectedIndentation)
-                                    )
+                        ) {
+                            val (actualIndent, actualContent) =
+                                if (it.isIndentBeforeClosingQuote()) {
+                                    it.text.splitIndentAt(it.text.length)
+                                } else if (it.isVariableStringTemplateEntry() && it.isFirstNonBlankElementOnLine()) {
+                                    it.getFirstElementOnSameLine().text.splitIndentAt(expectedPrefixLength)
                                 } else {
+                                    it.text.splitIndentAt(prefixLength)
+                                }
+                            if (indentConfig.containsUnexpectedIndentChar(actualIndent)) {
+                                val offsetFirstWrongIndentChar = indentConfig.indexOfFirstUnexpectedIndentChar(actualIndent)
+                                emit(
+                                    it.startOffset + offsetFirstWrongIndentChar,
+                                    "Unexpected '${indentConfig.unexpectedIndentCharDescription}' character(s) in margin of multiline string",
+                                    true
+                                )
+                                if (autoCorrect) {
                                     (it.firstChildNode as LeafPsiElement).rawReplaceWithText(
                                         expectedIndentation + actualContent
                                     )
                                 }
+                            } else if (actualIndent != expectedIndentation && it.isIndentBeforeClosingQuote()) {
+                                // It is a deliberate choice not to fix the indents inside the string literal except the line which only contains
+                                // the closing quotes.
+                                emit(
+                                    it.startOffset,
+                                    "Unexpected indent of multiline string closing quotes",
+                                    true
+                                )
+                                if (autoCorrect) {
+                                    if (it.firstChildNode == null) {
+                                        (it as LeafPsiElement).rawInsertBeforeMe(
+                                            LeafPsiElement(REGULAR_STRING_PART, expectedIndentation)
+                                        )
+                                    } else {
+                                        (it.firstChildNode as LeafPsiElement).rawReplaceWithText(
+                                            expectedIndentation + actualContent
+                                        )
+                                    }
+                                }
                             }
                         }
                     }
-                }
-        }
+            }
     }
 
     private fun KtStringTemplateExpression.isMultiLine(): Boolean {
@@ -1038,6 +1045,14 @@ class IndentationRule : Rule(
         return false
     }
 
+    private fun KtStringTemplateExpression.isFollowedByTrimIndent() = isFollowedBy("trimIndent()")
+
+    private fun KtStringTemplateExpression.isFollowedByTrimMargin() = isFollowedBy("trimMargin()")
+
+    private fun KtStringTemplateExpression.isFollowedBy(callExpressionName: String) =
+        this.node.nextSibling { it.elementType != ElementType.DOT }
+            .let { it?.elementType == CALL_EXPRESSION && it.text == callExpressionName }
+
     private fun String.indentLength() =
         indexOfFirst { !it.isWhitespace() }.let { if (it == -1) length else it }
 

diff --git a/...set-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt b/...set-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt
@@ -1314,7 +1314,7 @@ internal class IndentationRuleTest {
                     select u from User u
                     inner join Organization o on u.organization = o
                     where o = :organization
-                $MULTILINE_STRING_QUOTE)
+                $MULTILINE_STRING_QUOTE.trimIndent())
                 fun findByOrganization(organization: Organization, pageable: Pageable): Page<User>
             }
             """.trimIndent()
@@ -1326,7 +1326,7 @@ internal class IndentationRuleTest {
                     select u from User u
                     inner join Organization o on u.organization = o
                     where o = :organization
-                    $MULTILINE_STRING_QUOTE
+                    $MULTILINE_STRING_QUOTE.trimIndent()
                 )
                 fun findByOrganization(organization: Organization, pageable: Pageable): Page<User>
             }
@@ -1337,7 +1337,7 @@ internal class IndentationRuleTest {
             listOf(
                 LintError(line = 2, col = 12, ruleId = "indent", detail = "Missing newline after \"(\""),
                 LintError(line = 6, col = 1, ruleId = "indent", detail = "Unexpected indent of multiline string closing quotes"),
-                LintError(line = 6, col = 7, ruleId = "indent", detail = "Missing newline before \")\"")
+                LintError(line = 6, col = 20, ruleId = "indent", detail = "Missing newline before \")\"")
             )
         )
         assertThat(IndentationRule().format(code)).isEqualTo(formattedCode)
@@ -1714,6 +1714,55 @@ internal class IndentationRuleTest {
         assertThat(IndentationRule().format(code)).isEqualTo(code)
     }
 
+    @Test
+    fun `Issue 1375 - Do not format raw string literal when not followed by trimIndent or trimMargin`() {
+        val code =
+            """
+            val someCodeBlock = $MULTILINE_STRING_QUOTE
+              foo()$MULTILINE_STRING_QUOTE
+            """.trimIndent()
+        assertThat(IndentationRule().lint(code)).isEmpty()
+        assertThat(IndentationRule().format(code)).isEqualTo(code)
+    }
+
+    @Test
+    fun `Issue 1375 - Format raw string literal when followed by trimIndent`() {
+        val code =
+            """
+            val someCodeBlock = $MULTILINE_STRING_QUOTE
+              foo()$MULTILINE_STRING_QUOTE.trimIndent()
+            """.trimIndent()
+        val formattedCode =
+            """
+            val someCodeBlock = $MULTILINE_STRING_QUOTE
+              foo()
+            $MULTILINE_STRING_QUOTE.trimIndent()
+            """.trimIndent()
+        assertThat(IndentationRule().lint(code)).containsExactly(
+            LintError(2, 8, "indent", "Missing newline before \"\"\"")
+        )
+        assertThat(IndentationRule().format(code)).isEqualTo(formattedCode)
+    }
+
+    @Test
+    fun `Issue 1375 - Format raw string literal when followed by trimMargin`() {
+        val code =
+            """
+            val someCodeBlock = $MULTILINE_STRING_QUOTE
+              foo()$MULTILINE_STRING_QUOTE.trimMargin()
+            """.trimIndent()
+        val formattedCode =
+            """
+            val someCodeBlock = $MULTILINE_STRING_QUOTE
+              foo()
+            $MULTILINE_STRING_QUOTE.trimMargin()
+            """.trimIndent()
+        assertThat(IndentationRule().lint(code)).containsExactly(
+            LintError(2, 8, "indent", "Missing newline before \"\"\"")
+        )
+        assertThat(IndentationRule().format(code)).isEqualTo(formattedCode)
+    }
+
     private companion object {
         const val MULTILINE_STRING_QUOTE = "${'"'}${'"'}${'"'}"
         const val TAB = "${'\t'}"

diff --git a/...-ruleset-standard/src/test/resources/spec/indent/format-multiline-string-expected.kt.spec b/...-ruleset-standard/src/test/resources/spec/indent/format-multiline-string-expected.kt.spec
@@ -8,7 +8,11 @@ fun f0() {
     println("""${true}""")
     println(
         """
+"""
+    )
+    println(
         """
+        """.trimIndent()
     )
     f(
         """${
@@ -18,7 +22,7 @@ fun f0() {
 _${
         true
         }
-        """,
+        """.trimIndent(),
         """text"""
     )
 }
diff --git a/ktlint-ruleset-standard/src/test/resources/spec/indent/format-multiline-string.kt.spec b/ktlint-ruleset-standard/src/test/resources/spec/indent/format-multiline-string.kt.spec
@@ -6,11 +6,13 @@ fun f0() {
 println("""${true}""")
 println("""
 """)
+println("""
+""".trimIndent())
     f("""${
 true
     }
     text
 _${
 true
-    }""", """text""")
+    }""".trimIndent(), """text""")
 }
diff --git a/...et-standard/src/test/resources/spec/indent/format-raw-string-trim-indent-expected.kt.spec b/...et-standard/src/test/resources/spec/indent/format-raw-string-trim-indent-expected.kt.spec
@@ -110,6 +110,6 @@ ${Target(
         )}
 ${LoadStatement("@gmaven_rules//:gmaven.bzl", listOf("gmaven_rules"))}
 ${Target("gmaven_rules", listOf())}
-        """
+"""
 
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -110,6 +110,6 @@ ${Target( @@
             )}
     ${LoadStatement("@gmaven_rules//:gmaven.bzl", listOf("gmaven_rules"))}
     ${Target("gmaven_rules", listOf())}
-            """
+    """
     }