Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not indent string template starting at first position of line #2553

Merged
merged 3 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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()
}
}