Skip to content

Commit

Permalink
Do not indent string template starting at first position of line (#2553)
Browse files Browse the repository at this point in the history 
Also, the `indent` rule should not change unexpected indentation characters inside a string template but leave this up to the `string-template-indent` rule. As a result the `indent` rule could be a bit simplified.

Closes #2350
  • Loading branch information
@paul-dingemans
paul-dingemans authored Feb 14, 2024
1 parent a88a2e5 commit 8eb0a58
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 106 deletions.
101 changes: 12 additions & 89 deletions ...t-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IndentationRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.RETURN_KEYWORD
import com.pinterest.ktlint.rule.engine.core.api.ElementType.RPAR
import com.pinterest.ktlint.rule.engine.core.api.ElementType.SAFE_ACCESS_EXPRESSION
import com.pinterest.ktlint.rule.engine.core.api.ElementType.SECONDARY_CONSTRUCTOR
import com.pinterest.ktlint.rule.engine.core.api.ElementType.SHORT_STRING_TEMPLATE_ENTRY
import com.pinterest.ktlint.rule.engine.core.api.ElementType.STRING_TEMPLATE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.SUPER_TYPE_LIST
import com.pinterest.ktlint.rule.engine.core.api.ElementType.THEN
Expand Down Expand Up @@ -1046,7 +1045,7 @@ public class IndentationRule :
val nodeIndentLevel = indentConfig.indentLevelFrom(indentContext.nodeIndent)
val childIndentLevel = indentConfig.indentLevelFrom(indentContext.childIndent)
"Remove indent context with level ($nodeIndentLevel, $childIndentLevel) for ${indentContext.fromASTNode.elementType}: " +
"${indentContext.nodes}"
indentContext.nodes
}
indentContextStack
.removeLast()
Expand Down Expand Up @@ -1356,7 +1355,6 @@ private class StringTemplateIndenter(
return
}

val prefixLength = node.getCommonPrefixLength()
val prevLeaf = node.prevLeaf()
val correctedExpectedIndent =
if (codeStyle == ktlint_official && node.isRawStringLiteralReturnInFunctionBodyBlock()) {
Expand Down Expand Up @@ -1388,44 +1386,15 @@ private class StringTemplateIndenter(
}
node
.children()
.filter { it.isIndentBeforeClosingQuote() }
.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(correctedExpectedIndent.length)
} 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(
correctedExpectedIndent + actualContent,
)
}
} else if (actualIndent != correctedExpectedIndent && 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 (it.prevLeaf()?.text == "\n") {
val (actualIndent, actualContent) = it.text.splitIndentAt(it.text.length)
if (actualIndent != correctedExpectedIndent) {
// It is a deliberate choice not to fix the indents inside the string literal except the line which only
// contains the closing quotes. See 'string-template-indent` rule for fixing the content of the string
// template itself
emit(it.startOffset, "Unexpected indent of multiline string closing quotes", true)
if (autoCorrect) {
if (it.firstChildNode == null) {
(it as LeafPsiElement).rawInsertBeforeMe(
Expand Down Expand Up @@ -1453,29 +1422,6 @@ private class StringTemplateIndenter(

private fun ASTNode.isRawStringLiteralReturnInFunctionBodyBlock() = RETURN_KEYWORD == prevCodeLeaf()?.elementType

/**
* Get the length of the indent which is shared by all lines inside the string template except for the indent of
* the closing quotes.
*/
private fun ASTNode.getCommonPrefixLength() =
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

private fun KtStringTemplateExpression.isFollowedByTrimIndent() = isFollowedBy("trimIndent()")

private fun KtStringTemplateExpression.isFollowedByTrimMargin() = isFollowedBy("trimMargin()")
Expand Down Expand Up @@ -1521,13 +1467,6 @@ private class StringTemplateIndenter(
private fun ASTNode.isIndentBeforeClosingQuote() =
elementType == CLOSING_QUOTE || (text.isBlank() && nextCodeSibling()?.elementType == CLOSING_QUOTE)

private fun ASTNode.isLiteralStringTemplateEntry() = elementType == LITERAL_STRING_TEMPLATE_ENTRY && text != "\n"

private fun ASTNode.isVariableStringTemplateEntry() =
elementType == LONG_STRING_TEMPLATE_ENTRY || elementType == SHORT_STRING_TEMPLATE_ENTRY

private fun ASTNode.isClosingQuote() = elementType == CLOSING_QUOTE

private fun String.indentLength() = indexOfFirst { !it.isWhitespace() }.let { if (it == -1) length else it }

/**
Expand All @@ -1538,6 +1477,9 @@ private class StringTemplateIndenter(
*/
private fun String.splitIndentAt(index: Int): Pair<String, String> {
assert(index >= 0)
if (this == "\n") {
return Pair("", "")
}
val firstNonWhitespaceIndex =
indexOfFirst { !it.isWhitespace() }.let {
if (it == -1) {
Expand All @@ -1552,25 +1494,6 @@ private class StringTemplateIndenter(
second = this.substring(safeIndex),
)
}

private fun ASTNode.getFirstElementOnSameLine(): ASTNode {
val firstLeafOnLine = prevLeaf { it.text == "\n" }
return if (firstLeafOnLine == null) {
this
} else {
firstLeafOnLine.nextLeaf(includeEmpty = true) ?: this
}
}

private fun ASTNode.isFirstNonBlankElementOnLine(): Boolean {
var node: ASTNode? = getFirstElementOnSameLine()
while (node != null && node != this && node.text.isWhitespace()) {
node = node.nextLeaf()
}
return node != this
}

private fun String.isWhitespace() = none { !it.isWhitespace() }
}

public val INDENTATION_RULE_ID: RuleId = IndentationRule().ruleId
18 changes: 10 additions & 8 deletions ...d/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,20 +240,26 @@ public class StringTemplateIndentRule :
} else {
it.text.splitIndentAt(prefixLength)
}
val expectedIndent =
if (it.isIndentBeforeClosingQuote() || prefixLength > 0) {
newIndent
} else {
""
}
if (currentIndent.contains(wrongIndentChar)) {
checkAndFixWrongIndentationChar(
node = it,
oldIndent = currentIndent,
newIndent = newIndent,
newIndent = expectedIndent,
newContent = currentContent,
emit = emit,
autoCorrect = autoCorrect,
)
} else if (currentIndent != newIndent) {
} else if (currentIndent != expectedIndent) {
checkAndFixIndent(
node = it,
oldIndentLength = currentIndent.length,
newIndent = newIndent,
newIndent = expectedIndent,
newContent = currentContent,
autoCorrect = autoCorrect,
emit = emit,
Expand Down Expand Up @@ -314,11 +320,7 @@ public class StringTemplateIndentRule :
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
emit(
node.startOffset + oldIndentLength,
"Unexpected indent of raw string literal",
true,
)
emit(node.startOffset + oldIndentLength, "Unexpected indent of raw string literal", true)
if (autoCorrect) {
if (node.elementType == CLOSING_QUOTE) {
(node as LeafPsiElement).rawInsertBeforeMe(
Expand Down
43 changes: 34 additions & 9 deletions ...c/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/StringTemplateIndentRuleTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRuleBuilde
import com.pinterest.ktlint.test.LintViolation
import com.pinterest.ktlint.test.MULTILINE_STRING_QUOTE
import com.pinterest.ktlint.test.TAB
import com.pinterest.ktlint.test.replaceStringTemplatePlaceholder
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test

Expand Down Expand Up @@ -216,36 +217,35 @@ class StringTemplateIndentRuleTest {
}

@Test
fun `Format a multiline string literal with text starting at position 1 of the line`() {
fun `Format a multiline string literal with text starting at position 1 of the line then do not indent the content`() {
val code =
"""
fun foo() {
println($MULTILINE_STRING_QUOTE
Some text starting at the beginning of the line
Some text not starting at the beginning of the line
$MULTILINE_STRING_QUOTE.trimIndent()
)
$MULTILINE_STRING_QUOTE.trimIndent())
}
""".trimIndent()
val formattedCode =
"""
fun foo() {
println(
$MULTILINE_STRING_QUOTE
Some text starting at the beginning of the line
Some text starting at the beginning of the line
Some text not starting at the beginning of the line
Some text not starting at the beginning of the line
$MULTILINE_STRING_QUOTE.trimIndent()
)
}
""".trimIndent()
stringTemplateIndentRuleAssertThat(code)
.hasLintViolations(
.hasLintViolationsForAdditionalRule(
LintViolation(6, 1, "Unexpected indent of multiline string closing quotes"),
).hasLintViolations(
LintViolation(2, 13, "Expected newline before multiline string template"),
LintViolation(3, 1, "Unexpected indent of raw string literal"),
LintViolation(5, 1, "Unexpected indent of raw string literal"),
LintViolation(6, 1, "Unexpected indent of raw string literal"),
LintViolation(6, 17, "Expected newline after multiline string template"),
).isFormattedAs(formattedCode)
}

Expand Down Expand Up @@ -445,4 +445,29 @@ class StringTemplateIndentRuleTest {
.hasLintViolation(1, 11, "Expected newline before multiline string template")
.isFormattedAs(formattedCode)
}

@Test
fun `Issue 2530 - Given a raw string literal containing string templates at position 1 of the line`() {
// Interpret "$." in code sample below as "$". It is used whenever the code which has to be inspected should
// actually contain a string template. Using "$" instead of "$." would result in a String in which the string
// templates would have been evaluated before the code would actually be processed by the rule.
val code =
"""
fun foo() {
val strings = listOf("a")
println(
$MULTILINE_STRING_QUOTE
$.{strings.joinToString { "a" }}
$.strings
$MULTILINE_STRING_QUOTE
)
}
val foo =
$MULTILINE_STRING_QUOTE
$.{strings.joinToString { "a" }}
$.strings
$MULTILINE_STRING_QUOTE.trimIndent()
""".trimIndent().replaceStringTemplatePlaceholder()
stringTemplateIndentRuleAssertThat(code).hasNoLintViolations()
}
}

0 comments on commit 8eb0a58

Please sign in to comment.