From 826a25d8ea2db6eb947cbba21d1a2690da072ce7 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 21 Jun 2022 15:15:52 +0300 Subject: [PATCH 1/2] [#1347] Indent only `TEMPLATE_ENTRY` nodes immediately following a newline ### 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. --- .../rules/chapter3/files/IndentationRule.kt | 128 +++++++++++------- .../chapter3/spaces/IndentationRuleFixTest.kt | 17 --- .../spaces/IndentationRuleTestMixin.kt | 84 ++++++++++++ 3 files changed, 166 insertions(+), 63 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt index 7fe952fac8..1534738522 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt @@ -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 @@ -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 @@ -244,7 +246,9 @@ class IndentationRule(configRules: List) : 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, @@ -256,10 +260,7 @@ class IndentationRule(configRules: List) : 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) } } @@ -290,40 +291,60 @@ class IndentationRule(configRules: List) : 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 -> { + 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) } } @@ -368,9 +389,8 @@ class IndentationRule(configRules: List) : 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 @@ -381,20 +401,11 @@ class IndentationRule(configRules: List) : 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) @@ -539,6 +550,7 @@ class IndentationRule(configRules: List) : 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. @@ -548,6 +560,30 @@ class IndentationRule(configRules: List) : 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]. * diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleFixTest.kt index be35a8afaa..13dd478017 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleFixTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleFixTest.kt @@ -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 @@ -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) @@ -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) @@ -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)) @@ -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)) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTestMixin.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTestMixin.kt index a68af9f3b5..a9d7466cd9 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTestMixin.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/spaces/IndentationRuleTestMixin.kt @@ -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( @@ -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( From 1591d9ee597132e2f4d1892f643dc43c6b5f73d6 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Tue, 21 Jun 2022 20:23:35 +0300 Subject: [PATCH 2/2] [#1347] Indent only `TEMPLATE_ENTRY` nodes immediately following a newline ### What's done: * Minor code clean-up. --- .../diktat/ruleset/rules/chapter3/files/IndentationRule.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt index 1534738522..afd0b56fcf 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/files/IndentationRule.kt @@ -291,7 +291,7 @@ class IndentationRule(configRules: List) : DiktatRule( actualIndent: Int ) { val templateEntries = stringTemplate.getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY) - val templateEntriesSize = templateEntries.size + val templateEntriesLastIndex = templateEntries.size - 1 var templateEntryFollowingNewline = false templateEntries.forEachIndexed { index, templateEntry -> @@ -321,7 +321,7 @@ class IndentationRule(configRules: List) : DiktatRule( * This is the last string template fragment which is usually followed * with the closing `"""` and the `.trimIndent()` or `.trimMargin(...)` call. */ - index == templateEntriesSize - 1 -> { + index == templateEntriesLastIndex -> { val lastRegularStringPart = templateEntries.last().firstChildNode as LeafPsiElement lastRegularStringPart.checkRegularStringPart().apply { val textWithoutIndent = text.trimStart()