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

[#1347] Indent only TEMPLATE_ENTRY nodes immediately following a newline #1387

Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -37,6 +37,7 @@ import com.pinterest.ktlint.core.ast.ElementType.CLOSING_QUOTE
import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.ELSE
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import com.pinterest.ktlint.core.ast.ElementType.LBRACE
import com.pinterest.ktlint.core.ast.ElementType.LBRACKET
import com.pinterest.ktlint.core.ast.ElementType.LITERAL_STRING_TEMPLATE_ENTRY
Expand All @@ -46,6 +47,7 @@ import com.pinterest.ktlint.core.ast.ElementType.LONG_TEMPLATE_ENTRY_START
import com.pinterest.ktlint.core.ast.ElementType.LPAR
import com.pinterest.ktlint.core.ast.ElementType.RBRACE
import com.pinterest.ktlint.core.ast.ElementType.RBRACKET
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.REGULAR_STRING_PART
import com.pinterest.ktlint.core.ast.ElementType.RPAR
import com.pinterest.ktlint.core.ast.ElementType.SAFE_ACCESS_EXPRESSION
Expand Down Expand Up @@ -244,7 +246,9 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
}

/**
* Checks if it is triple-quoted string template with trimIndent() or trimMargin() function.
* Checks if it is a triple-quoted string template with
* [trimIndent()][String.trimIndent] or [trimMargin(...)][String.trimMargin]
* function.
*/
private fun checkStringLiteral(
whiteSpace: PsiWhiteSpace,
Expand All @@ -256,10 +260,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
nextNodeDot.elementType == DOT_QUALIFIED_EXPRESSION &&
nextNodeDot.firstChildNode.elementType == STRING_TEMPLATE &&
nextNodeDot.firstChildNode.text.startsWith("\"\"\"") &&
nextNodeDot.findChildByType(CALL_EXPRESSION)?.text?.let {
it == "trimIndent()" ||
it == "trimMargin()"
} == true) {
nextNodeDot.findChildByType(CALL_EXPRESSION).isTrimIndentOrMarginCall()) {
fixStringLiteral(nextNodeDot.firstChildNode, expectedIndent, actualIndent)
}
}
Expand Down Expand Up @@ -290,40 +291,60 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
actualIndent: Int
) {
val templateEntries = stringTemplate.getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY)
templateEntries.asSequence().filterIndexed { index, templateEntry ->
val templateEntriesSize = templateEntries.size
var templateEntryFollowingNewline = false

templateEntries.forEachIndexed { index, templateEntry ->
val text = templateEntry.text
val containsNewline = text.contains(NEWLINE)

if (containsNewline) {
when {
text.contains(NEWLINE) -> {
/*
* Set the flag.
*/
templateEntryFollowingNewline = true

/*
* In real-life cases observed, whenever a `LITERAL_STRING_TEMPLATE_ENTRY`
* _contains_ a newline character, it is _exactly_ a newline character.
*/
check(text.length == 1) {
val escapedText = text.replace(NEWLINE.toString(), "\\n")

"A LITERAL_STRING_TEMPLATE_ENTRY at index $index contains extra characters in addition to the newline, " +
"entry: \"$escapedText\", " +
"string template: ${stringTemplate.text}"
}
}

/*
* In real-life cases observed, whenever a `LITERAL_STRING_TEMPLATE_ENTRY`
* _contains_ a newline character, it is _exactly_ a newline character.
* This is the last string template fragment which is usually followed
* with the closing `"""` and the `.trimIndent()` or `.trimMargin(...)` call.
*/
check(text.length == 1) {
val escapedText = text.replace(NEWLINE.toString(), "\\n")
index == templateEntriesSize - 1 -> {
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
val lastRegularStringPart = templateEntries.last().firstChildNode as LeafPsiElement
lastRegularStringPart.checkRegularStringPart().apply {
val textWithoutIndent = text.trimStart()
rawReplaceWithText(expectedIndent.spaces + textWithoutIndent)
}
}

"A LITERAL_STRING_TEMPLATE_ENTRY at index $index contains extra characters in addition to the newline, " +
"entry: \"$escapedText\", " +
"string template: ${stringTemplate.text}"
/*
* Either this is the very first string template entry, or an
* entry which immediately follows the newline.
*/
index == 0 || templateEntryFollowingNewline -> {
fixFirstTemplateEntries(
templateEntry,
expectedIndent = expectedIndent,
actualIndent = actualIndent)

/*
* Re-set the flag.
*/
templateEntryFollowingNewline = false
}
}

!containsNewline
}.forEach { templateEntry ->
fixFirstTemplateEntries(
templateEntry,
expectedIndent = expectedIndent,
actualIndent = actualIndent)
}

/*
* This is the last string template fragment which is usually followed
* with the closing `"""` and the `.trimIndent()` or `.trimMargin()` call.
*/
val lastRegularStringPart = templateEntries.last().firstChildNode as LeafPsiElement
lastRegularStringPart.checkRegularStringPart().apply {
val textWithoutIndent = text.trimStart()
rawReplaceWithText(expectedIndent.spaces + textWithoutIndent)
}
}

Expand Down Expand Up @@ -368,9 +389,8 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
*/
val regularStringPart = templateEntry.firstChildNode as LeafPsiElement
val regularStringPartText = regularStringPart.checkRegularStringPart().text
val nodeStartIndentOrNegative = (regularStringPartText.leadingSpaceCount() - actualIndent).unindent()
// shift of the node depending on its initial string template indent
val nodeStartIndent = nodeStartIndentOrNegative.zeroIfNegative()
val nodeStartIndent = (regularStringPartText.leadingSpaceCount() - actualIndent).unindent().zeroIfNegative()

val isPrevStringTemplate = templateEntry.treePrev.elementType in stringLiteralTokens
val isNextStringTemplate = templateEntry.treeNext.elementType in stringLiteralTokens
Expand All @@ -381,20 +401,11 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(

// if string template is before literal_string
else -> regularStringPartText.trimEnd()

}

else -> {
val textIndent = expectedIndent.indent().spaces

when {
// if string template is after literal_string
isNextStringTemplate -> textIndent + nodeStartIndent.spaces + regularStringPartText.trimStart()

// if there is no string template in literal_string
else -> textIndent + nodeStartIndent.spaces + regularStringPartText.trim()
}
}
// if string template is after literal_string
// or if there is no string template in literal_string
else -> (expectedIndent.indent() + nodeStartIndent).spaces + regularStringPartText.trimStart()
}

regularStringPart.rawReplaceWithText(correctedText)
Expand Down Expand Up @@ -539,6 +550,7 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
private val decreasingTokens = listOf(RPAR, RBRACE, RBRACKET, LONG_TEMPLATE_ENTRY_END)
private val matchingTokens = increasingTokens.zip(decreasingTokens)
private val stringLiteralTokens = listOf(SHORT_STRING_TEMPLATE_ENTRY, LONG_STRING_TEMPLATE_ENTRY)
private val knownTrimFunctionPatterns = setOf("trimIndent", "trimMargin")

/**
* @return a string which consists of `N` [space][SPACE] characters.
Expand All @@ -548,6 +560,30 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
get() =
SPACE.toString().repeat(n = this)

/**
* @return `true` if this is a [String.trimIndent] or [String.trimMargin]
* call, `false` otherwise.
*/
private fun ASTNode?.isTrimIndentOrMarginCall(): Boolean {
this ?: return false

require(elementType == CALL_EXPRESSION) {
"The elementType of this node is $elementType while $CALL_EXPRESSION expected"
}

val referenceExpression = firstChildNode ?: return false
if (referenceExpression.elementType != REFERENCE_EXPRESSION) {
return false
}

val identifier = referenceExpression.firstChildNode ?: return false
if (identifier.elementType != IDENTIFIER) {
return false
}

return identifier.text in knownTrimFunctionPatterns
}

/**
* Checks this [REGULAR_STRING_PART] child of a [LITERAL_STRING_TEMPLATE_ENTRY].
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import org.cqfn.diktat.util.FixTestBase
import generated.WarningNames
import org.assertj.core.api.SoftAssertions.assertSoftly
import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.Assumptions.assumeTrue
import org.junit.jupiter.api.MethodOrderer.MethodName
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -104,10 +103,6 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
@Test
@Tag(WarningNames.WRONG_INDENTATION)
fun `expression body functions should be reformatted if mis-indented (extendedIndentAfterOperators = true)`(@TempDir tempDir: Path) {
assumeTrue(testsCanBeMuted()) {
"Skipping a known-to-fail test"
}

val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentAfterOperators" to true)

Expand All @@ -126,10 +121,6 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
@Test
@Tag(WarningNames.WRONG_INDENTATION)
fun `expression body functions should be reformatted if mis-indented (extendedIndentAfterOperators = false)`(@TempDir tempDir: Path) {
assumeTrue(testsCanBeMuted()) {
"Skipping a known-to-fail test"
}

val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters("extendedIndentAfterOperators" to false)

Expand Down Expand Up @@ -176,10 +167,6 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
@Test
@Tag(WarningNames.WRONG_INDENTATION)
fun `no whitespace should be injected into multi-line string literals (mis-indented code reformatted, extendedIndent = true)`(@TempDir tempDir: Path) {
assumeTrue(testsCanBeMuted()) {
"Skipping a known-to-fail test"
}

val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters(*extendedIndent(enabled = true))

Expand All @@ -196,10 +183,6 @@ class IndentationRuleFixTest : FixTestBase("test/paragraph3/indentation",
@Test
@Tag(WarningNames.WRONG_INDENTATION)
fun `no whitespace should be injected into multi-line string literals (mis-indented code reformatted, extendedIndent = false)`(@TempDir tempDir: Path) {
assumeTrue(testsCanBeMuted()) {
"Skipping a known-to-fail test"
}

val defaultConfig = IndentationConfig("newlineAtEnd" to false)
val customConfig = defaultConfig.withCustomParameters(*extendedIndent(enabled = false))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,48 @@ internal interface IndentationRuleTestMixin {
|}
""".trimMargin(),

"""
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
|fun f0() {
| @Language("kotlin")
| val code =
| ""${'"'}
| |@Suppress("diktat")
| |fun foo() {
| | val a = 1
| |}
| ""${'"'}.trimMargin()
| lintMethod(code)
|}
""".trimMargin(),

"""
|fun f1() {
| @Language("kotlin")
| val code =
| ""${'"'}
| |@Suppress("diktat")
| |fun foo() {
| | val a = 1
| |}
| ""${'"'}.trimMargin("|")
| lintMethod(code)
|}
""".trimMargin(),

"""
|fun f2() {
| @Language("kotlin")
| val code =
| ""${'"'}
| >@Suppress("diktat")
| >fun foo() {
| > val a = 1
| >}
| ""${'"'} . trimMargin ( marginPrefix = ">" )
| lintMethod(code)
|}
""".trimMargin(),

"""
|fun checkScript() {
| lintMethod(
Expand Down Expand Up @@ -193,6 +235,48 @@ internal interface IndentationRuleTestMixin {
|}
""".trimMargin(),

"""
|fun f0() {
| @Language("kotlin")
| val code =
| ""${'"'}
| |@Suppress("diktat")
| |fun foo() {
| | val a = 1
| |}
| ""${'"'}.trimMargin()
| lintMethod(code)
|}
""".trimMargin(),

"""
|fun f1() {
| @Language("kotlin")
| val code =
| ""${'"'}
| |@Suppress("diktat")
| |fun foo() {
| | val a = 1
| |}
| ""${'"'}.trimMargin("|")
| lintMethod(code)
|}
""".trimMargin(),

"""
|fun f2() {
| @Language("kotlin")
| val code =
| ""${'"'}
| >@Suppress("diktat")
| >fun foo() {
| > val a = 1
| >}
| ""${'"'} . trimMargin ( marginPrefix = ">" )
| lintMethod(code)
|}
""".trimMargin(),

"""
|fun checkScript() {
| lintMethod(
Expand Down