From 1cc6e347944db20d34442b93147d630272f81535 Mon Sep 17 00:00:00 2001 From: paul-dingemans Date: Fri, 25 Feb 2022 20:55:16 +0100 Subject: [PATCH] Do not indent raw string literals that are not followed by either trimIndent() or trimMargin() Closes #1375 --- CHANGELOG.md | 1 + .../ruleset/standard/IndentationRule.kt | 193 ++++++++++-------- .../ruleset/standard/IndentationRuleTest.kt | 55 ++++- .../format-multiline-string-expected.kt.spec | 6 +- .../indent/format-multiline-string.kt.spec | 4 +- ...at-raw-string-trim-indent-expected.kt.spec | 2 +- 6 files changed, 166 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e32e1c8fae..921d050aff 100644 --- 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/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRule.kt index bb56bfc35e..0c2b274825 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRule.kt +++ b/ktlint-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/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt index e8ead4edb9..3a20998bbf 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt +++ b/ktlint-ruleset-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 } """.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 } @@ -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/ktlint-ruleset-standard/src/test/resources/spec/indent/format-multiline-string-expected.kt.spec b/ktlint-ruleset-standard/src/test/resources/spec/indent/format-multiline-string-expected.kt.spec index 00ff2f7cc7..a2cb9d8a27 100644 --- a/ktlint-ruleset-standard/src/test/resources/spec/indent/format-multiline-string-expected.kt.spec +++ b/ktlint-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 index 190d39f4ab..17b821deb3 100644 --- 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/ktlint-ruleset-standard/src/test/resources/spec/indent/format-raw-string-trim-indent-expected.kt.spec b/ktlint-ruleset-standard/src/test/resources/spec/indent/format-raw-string-trim-indent-expected.kt.spec index 90391fa2cc..79ea8cc075 100644 --- a/ktlint-ruleset-standard/src/test/resources/spec/indent/format-raw-string-trim-indent-expected.kt.spec +++ b/ktlint-ruleset-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())} - """ +""" }