Skip to content

Commit

Permalink
[#1347] Indent only TEMPLATE_ENTRY nodes immediately following a ne…
Browse files Browse the repository at this point in the history 
…wline (#1387)

### What's done:

 * Now, only those `LITERAL_STRING_TEMPLATE_ENTRY` nodes which immediately
   follow a newline are indented.
 * Additionally, with this change, `trimIndent()` and `trimMargin(...)` calls
   are more reliably detected.
 * This fixes #1347.
  • Loading branch information
@0x6675636b796f75676974687562
0x6675636b796f75676974687562 authored Jun 21, 2022
1 parent d96621e commit c0b42a8
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 63 deletions.
128 changes: 82 additions & 46 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt
View file
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 templateEntriesLastIndex = templateEntries.size - 1
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 == templateEntriesLastIndex -> {
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
17 changes: 0 additions & 17 deletions ...t-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleFixTest.kt
View file
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
84 changes: 84 additions & 0 deletions ...rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTestMixin.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,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 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

0 comments on commit c0b42a8

Please sign in to comment.